[Python-bugs-list] [Bug #119486] fcntl.lockf() is broken

noreply@sourceforge.net noreply@sourceforge.net
Tue, 19 Dec 2000 17:19:36 -0800


Bug #119486, was updated on 2000-Oct-26 14:18
Here is a current snapshot of the bug.

Project: Python
Category: Extension Modules
Status: Open
Resolution: None
Bug Group: None
Priority: 5
Submitted by: flight
Assigned to : gvanrossum
Summary: fcntl.lockf() is broken

Details: Another observation by James Troup <troup@debian.org>: fcntl.lockf()
seems to be severly `broken': it's acting like flock, not like lockf, 
and the code seems to be a copy/paste of flock.
(registered in the Debian Bug Tracking System as bug #74777,
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=74777).

James includes a first start at filling in a correct lockf function.
Looks like it needs some more work, therefore I don't submit it as
patch. The patch is against 1.5.2, though there seem to be no changes in 2.0.

These are James' words:


fcntl.lockf() doesn't work as expected.  

        fcntl.lockf(fd, FCNTL.F_TLOCK);

will block.. looking at the source is exteremly confusing.
fcntl.lockf() appears to want flock() style arguments?!  It almost
looks like someone cut'n'wasted from the fcntl_flock() function just
above...

Anyway, here is a patch which is IMO the Right Thing,
i.e. fcnt.lockf() acting like libc lockf() and like it's documented to
do...

--- python-1.5.2/Modules/fcntlmodule.c.orig     Sat Oct 14 15:46:40 2000
+++ python-1.5.2/Modules/fcntlmodule.c  Sat Oct 14 18:31:44 2000
@@ -233,30 +233,12 @@
 {
        int fd, code, ret, whence = 0;
        PyObject *lenobj = NULL, *startobj = NULL;
+       struct flock l;
 
        if (!PyArg_ParseTuple(args, "ii|OOi", &fd, &code,
                              &lenobj, &startobj, &whence))
            return NULL;
 
-#ifndef LOCK_SH
-#define LOCK_SH                1       /* shared lock */
-#define LOCK_EX                2       /* exclusive lock */
-#define LOCK_NB                4       /* don't block when locking */
-#define LOCK_UN                8       /* unlock */
-#endif
-       {
-               struct flock l;
-               if (code == LOCK_UN)
-                       l.l_type = F_UNLCK;
-               else if (code & LOCK_SH)
-                       l.l_type = F_RDLCK;
-               else if (code & LOCK_EX)
-                       l.l_type = F_WRLCK;
-               else {
-                       PyErr_SetString(PyExc_ValueError,
-                                       "unrecognized flock argument");
-                       return NULL;
-               }
                l.l_start = l.l_len = 0;
                if (startobj != NULL) {
 #if !defined(HAVE_LARGEFILE_SUPPORT)
@@ -281,10 +263,48 @@
                                return NULL;
                }
                l.l_whence = whence;
+       switch (code)
+         {
+         case F_TEST:
+           /* Test the lock: return 0 if FD is unlocked or locked by this process;
+              return -1, set errno to EACCES, if another process holds the lock.  */
+           if (fcntl (fd, F_GETLK, &l) < 0) {
+             fprintf(stderr, "andrea: 1");
+             PyErr_SetFromErrno(PyExc_IOError);
+             return NULL;
+           }
+           if (l.l_type == F_UNLCK || l.l_pid == getpid ()) {
+             fprintf(stderr, "andrea: 2");
+             Py_INCREF(Py_None);
+             return Py_None;
+           }
+           fprintf(stderr, "andrea: 3");
+           errno = EACCES;
+           PyErr_SetFromErrno(PyExc_IOError);
+           return NULL;
+
+         case F_ULOCK:
+           l.l_type = F_UNLCK;
+           code = F_SETLK;
+           break;
+         case F_LOCK:
+           l.l_type = F_WRLCK;
+           code = F_SETLKW;
+           break;
+         case F_TLOCK:
+           l.l_type = F_WRLCK;
+           code = F_SETLK;
+           break;
+           
+         default:
+           PyErr_SetString(PyExc_ValueError,
+                           "unrecognized flock argument");
+           return NULL;
+         }
                Py_BEGIN_ALLOW_THREADS
-               ret = fcntl(fd, (code & LOCK_NB) ? F_SETLK : F_SETLKW, &l);
+         ret = fcntl(fd, code, &l);
                Py_END_ALLOW_THREADS
-       }
+
        if (ret < 0) {
                PyErr_SetFromErrno(PyExc_IOError);
                return NULL;



Follow-Ups:

Date: 2000-Dec-19 17:19
By: akuchling

Comment:
Note that the docs say that lockf "is a wrapper around the \constant{FCNTL.F_SETLK} and
\constant{FCNTL.F_SETLKW} \function{fcntl()} calls."  Stevens's 
"Advanced Programming in the Unix Env." concurs, on page 367: "The System V lockf() is just an interface to fcntl()."

However, none of us are serious Unix weenies.   Unfortunately, the documented Python lockf() provides features above the libc lockf(), so
using the system lockf() seems impossible unless we break backwards compatibility.   Unfortunately none of us are serious Unix weenies, so writing a lockf() emulation that's completely correct everywhere might be very difficult.

Troup's patch attempts to fix the emulation; I haven't looked at it closely.  I'd suggest breaking backwards compatibility; if that's out, 
we should take a close look at the patch.  

-------------------------------------------------------

Date: 2000-Nov-24 01:32
By: sjoerd

Comment:
Yeah, I looked at it.  In 1996.  I don't think I looked at flock since then.
Anyway, it compiles on my system (IRIX 6.5.2), and I don't think I will have time in the near future to look any further into this.
-------------------------------------------------------

Date: 2000-Oct-26 14:29
By: gvanrossum

Comment:
Sjoerd, you once looked at this code. Can you comment on this? If you don't have time, please assign back to me.
-------------------------------------------------------

For detailed info, follow this link:
http://sourceforge.net/bugs/?func=detailbug&bug_id=119486&group_id=5470