Current file locking does not work on NFS mounts

[Malcolm Tredinnick]

The code in django.core.files.locks assumes that flock() is safe to use on every POSIX system to lock files. This isn't the case when the filesystem in question is a relatively recent NFS system. In that case, flock() actively raises an error (rather than potentially silently failing to lock things, which was the older behaviour). For those situations fnctl() should be used to do file locking.

From a bit of a search around and some source reading (plus some comments on the Mailman list, although I've avoided reading the relevant code there for the moment, since it's GPL and I might end up being the person who fixes this ticket), it looks like posixfile.lock() in the Python source is an appropriate way to do this.

I realise the locks.py file isn't our source, but it is broken in this respect and we do include it. So we have to fix it, since, right now, you can't save files to an NFS-mounted filesystem.

This effect in Django was initially noticed in ​this django-users thread.

[roman]

The file of interest is django/core/files/locks.py.

Imported in django/core/files/storage.py django/core/files/move.py

Pointlessly imported in django/db/models/base.py

Trying to change to fcntl.fcntl() from fcntl.flock() in the lock and unlock. Using posfixfile.lock() looks like the best way forward, but I get an error

   File "/usr/lib/python2.5/site-packages/django/core/files/locks.py", line 70, in lock
    rv = fcntl.fcntl(file, fcntl.F_SETLKW, lockdata)
IOError: [Errno 22] Invalid argument

This guy had the same NFS locking problem and says he got it to work. It did not work for me. Then again, my NFS mount seemed to support flock() even with subtree_check.
​http://coding.derkeiler.com/Archive/Python/comp.lang.python/2007-06/msg00801.html

[roman]

Test using locking, run in two separate terminals, but in the same directory.

[Malcolm Tredinnick]

Okay, this turns out to be trickier to replicate than it looks at first sight. It's pretty dependent on the OS, and the NFS client and server versions, and the export options for the NFS mount. In fact, I wasn't able to replicate it on my laptop or two other systems with proper remote NFS mounts that I had access to.

With a bit of reading of the Python source and some thinking about what's going on, I think I've solved it in a portable fashion. So the following commit will close this ticket off. If somebody can subsequently repeat the problem, please do reopen this, but we'll need all of the above information: OS, export options, nfsd and client nfs tools versions (where appropriate). I'd also like to know if you're able to lock a file opened for writing on your particular system using Python's fcntl module in any fashion, because we'd be down to writing workarounds at that level.

By the way, the use of the posixfile module doesn't seem appropriate, after looking a bit more closely. It's pretty clearly marked as deprecated and would be bad to introduce a dependency on it now.

[Malcolm Tredinnick]

(In [8675]) Fixed #8403 -- Changed the use of fcntl.flock() to fcntl.lockf(). On some
systems, this will ensure fnctl-based file locking is always used, which means
locking of NFS-mounted files should work.

After updating to Django 1.0 I got this traceback where the admin tries to write stuff to a NFS mount. I'm not 100% sure it's the the same issue that this ticket was about, but it fails at the line of the fix ...

• OS: Debian Etch
• export options: (rw,sync,no_subtree_check)
• nfsd and client nfs tools versions: 1.0.10-6+etch.1
• options from the mount: rw,vers=3,rsize=32768,wsize=32768,hard,intr,proto=tcp,timeo=600,retrans=2,sec=sys,addr=vm1 0 0

Traceback:

Traceback:
File "/usr/lib/python2.4/site-packages/django/core/handlers/base.py" in get_response
  86.                 response = callback(request, *callback_args, **callback_kwargs)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/sites.py" in root
  158.                 return self.model_page(request, *url.split('/', 2))
File "/usr/lib/python2.4/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  44.         response = view_func(request, *args, **kwargs)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/sites.py" in model_page
  177.         return admin_obj(request, rest_of_url)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/options.py" in __call__
  197.             return self.change_view(request, unquote(url))
File "/usr/lib/python2.4/site-packages/django/db/transaction.py" in _commit_on_success
  238.                 res = func(*args, **kw)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/options.py" in change_view
  579.                 new_object = self.save_form(request, form, change=True)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/options.py" in save_form
  370.         return form.save(commit=False)
File "/usr/lib/python2.4/site-packages/django/forms/models.py" in save
  302.         return save_instance(self, self.instance, self._meta.fields, fail_message, commit)
File "/usr/lib/python2.4/site-packages/django/forms/models.py" in save_instance
  47.         f.save_form_data(instance, cleaned_data[f.name])
File "/usr/lib/python2.4/site-packages/django/db/models/fields/files.py" in save_form_data
  192.             getattr(instance, self.name).save(data.name, data, save=False)
File "/usr/lib/python2.4/site-packages/django/db/models/fields/files.py" in save
  217.         super(ImageFieldFile, self).save(name, content, save)
File "/usr/lib/python2.4/site-packages/django/db/models/fields/files.py" in save
  74.         self._name = self.storage.save(name, content)
File "/usr/lib/python2.4/site-packages/django/core/files/storage.py" in save
  45.         name = self._save(name, content)
File "/usr/lib/python2.4/site-packages/django/core/files/storage.py" in _save
  150.                     file_move_safe(content.temporary_file_path(), full_path)
File "/usr/lib/python2.4/site-packages/django/core/files/move.py" in file_move_safe
  70.             locks.lock(fd, locks.LOCK_EX)
File "/usr/lib/python2.4/site-packages/django/core/files/locks.py" in lock
  57.         fcntl.lockf(fd(file), flags)

Exception Type: IOError at /program/event/1109/
Exception Value: [Errno 5] Input/output error

Let me know if I should post more info ...

[Malcolm Tredinnick]

comment:7 looks like a slightly different problem. Since we've fixed the reason for the first failure, I've created a new ticket for the second version (#9099), rather than having this one reopened with every different instance of NFS problems. That will help keep the history straight.

[rndblnch]

I reopen this bug because the fix introduces a regression for my specific case :

• system : macosx
• django : 1.0
• filesystem accessed : afp (it works other with on the local file system, no idea over nfs)

Reverting Changeset 8675 solves the problem ...

[Karen Tracey]

Changeset [8675] fixed a real problem and isn't simply going to be reverted. If it's causing a problem on your setup, please open a new ticket to address that. In the new ticket please include specifics of the "regression" -- what exactly happens? You have not provided enough information here for anyone to solve "the problem" you are seeing, please provide more specifics of the problem you are seeing in the new ticket.

[rndblnch]

Replying to kmtracey:

Changeset [8675] fixed a real problem and isn't simply going to be reverted. If it's causing a problem on your setup, please open a new ticket to address that. In the new ticket please include specifics of the "regression" -- what exactly happens? You have not provided enough information here for anyone to solve "the problem" you are seeing, please provide more specifics of the problem you are seeing in the new ticket.

done here: #9433

[Wolph]

In case anyone still has problems with this (I had when mounting a Linux NFS share from a FreeBSD system), with "-L" you can enable local locking so the machine makes sure all the locks work locally but they won't get communicated with the server.

Without that option you will have to run lockd on both the client and server to transfer the locks, that way the locks will be shared by all clients.

[Jacob]

Milestone 1.0 deleted