To reproduce:
1. Install file lock module
2. Lock a file
3. Uninstall the module
4. Try to delete the locked file

What happen:
The file is still locked

Expected:
File unlocked since the module has been uninstalled

Maybe related:
File: file_lock.module
Line: 171
The ID is a string instead of the reference to the constant.
file_usage_delete($file, 'file_lock', 'FILE_LOCK_ID');
should be
file_usage_delete($file, 'file_lock', FILE_LOCK_ID);

CommentFileSizeAuthor
#7 file_lock-2107925-uninstall-file-unlock-7.patch741 bytesloopduplicate
#2 file_lock-2107925-uninstall-file-unlock.patch3.65 KBgaellafond

Comments

gaellafond’s picture

gaellafond commented

Solution:
File: file_lock.install

function file_lock_uninstall() {
  variable_del('file_lock_mode');
  variable_del('file_lock_pattern');
  variable_del('file_lock_regex');
  variable_del('file_lock_hook');
}

changed to:

function file_lock_uninstall() {
  variable_del('file_lock_mode');
  variable_del('file_lock_pattern');
  variable_del('file_lock_regex');
  variable_del('file_lock_hook');
  $files = file_load_multiple(FALSE);
  foreach ($files as $file) {
    file_usage_delete($file, 'file_lock', 0);
  }
}

If you want to use the function "file_lock_remove_lock" instead of file_usage_delete (which would be a good idea), you should define "file_lock_remove_lock" in a separate file (file_lock.inc) that you include in the .module and the .install using:
require_once (dirname(__FILE__) . '/file_lock.inc');
The constant "FILE_LOCK_ID" should also be defined in that file as well, and the "file_lock_add_lock" for consistency.

gaellafond’s picture

gaellafond commented
StatusFileSize
new file_lock-2107925-uninstall-file-unlock.patch3.65 KB

Patch
* Remove lock on all locked files, when uninstall is run,
* Added a file_lock.inc containing common functions,
* Added a list of module files and the link to the module config page in the .info

gaellafond’s picture

gaellafond commented
Issue summary: View changes

Better formating

xtfer’s picture

xtfer commented
Category: Bug report » Feature request
Issue summary: View changes
mikeytown2’s picture

mikeytown2 commented
Status: Active » Needs review
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

+1 for RTBC from my side. Important patch that works good for me!

mikeytown2’s picture

mikeytown2 commented
Status: Needs review » Reviewed & tested by the community
loopduplicate’s picture

loopduplicate commented
Category: Feature request » Bug report
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new file_lock-2107925-uninstall-file-unlock-7.patch741 bytes

Hi All,

I changed this to a bug because it seems like leaving behind stale file usage entries after uninstall is a bug. Feel free to disagree and change back.

The patch in #2 no longer applies, so I'm uploading a revised patch as well as making a few comments about the patch in #2:

+++ b/file_lock.inc
@@ -0,0 +1,58 @@
+<?php

The new patch I'm uploading does not include a new include file.

+++ b/file_lock.info
@@ -3,3 +3,9 @@ description = Prevent deletion of untracked files from file system.
+files[] = file_lock.module
+files[] = file_lock.inc
+files[] = file_lock.admin.inc

There is no need to add these to the .info files array:

  • .module files never need to be added
  • The second file doesn't exist in the new patch and didn't have any classes to autoload
  • file_lock.admin.inc doesn't contain any classes that need to be autoloaded
+++ b/file_lock.inc
@@ -0,0 +1,58 @@
+/**
+ * Remove lock for the $file
+ *
+ * @param file $file
+ *   file object (with at least 'fid' set)
+ */
...
+  file_usage_delete($file, 'file_lock', FILE_LOCK_ID);
+}
+
+/**
+ * Remove lock for all locked files
+ */
+function file_lock_remove_all_lock() {
+  $files = file_load_multiple(FALSE);
+  foreach ($files as $file) {
+    file_lock_remove_lock($file);
+  }
+}

In the new patch, instead of using these two functions, hook_uninstall() uses the code from those functions inline. Incidentally, file_lock_remove_lock() was removed from this module since the time that the patch in #2 was made.

+++ b/file_lock.install
@@ -14,6 +15,7 @@ function file_lock_uninstall() {
+  file_lock_remove_all_lock();

The new patch doesn't call an external function, instead it has a few lines of inline code

Regards,
loop

aimeerae’s picture

aimeerae
she/her
Primary language English
Location San Francisco, CA
commented
Status: Needs review » Reviewed & tested by the community

Hi loop, I tested your patch in https://www.drupal.org/node/2107925#comment-11280851 and the file locks are deleted after the uninstall of the module.

Note: files that are in use by other modules (node, etc) are still locked, as they should be.

Files that were only locked by the file_lock module can be deleted after the module is not just disabled, but uninstalled.