[Python-bugs-list] [ python-Bugs-219486 ] fcntl.lockf() is broken

noreply@sourceforge.net noreply@sourceforge.net
Thu, 20 Dec 2001 20:36:13 -0800


Bugs item #219486, was opened at 2000-10-26 14:18
You can respond by visiting: 
http://sourceforge.net/tracker/?func=detail&atid=105470&aid=219486&group_id=5470

Category: Extension Modules
Group: Not a Bug
>Status: Open
Resolution: Wont Fix
Priority: 5
Submitted By: Gregor Hoffleit (flight)
Assigned to: Guido van Rossum (gvanrossum)
Summary: fcntl.lockf() is broken

Initial Comment:
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;



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

>Comment By: Guido van Rossum (gvanrossum)
Date: 2001-12-20 20:36

Message:
Logged In: YES 
user_id=6380

doko, what's going on here? I don't appreciate your
inserting foul language in our tracker.

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

Comment By: Matthias Klose (doko)
Date: 2001-12-20 16:38

Message:
Logged In: YES 
user_id=60903

[repeat a final (?) comment from the bug submitter. please 
CC 74777-submitter@bugs.debian.org on replies.

74777-submitter writes:

So...

Executive summary:

 o yes, python's lockf() is fucked

 o no, there's not a good reason for it, just the lame ass 
excuse that
   they were tying to make things easier (??)

 o No, they won't fix it because that would mean breaking 
backward
   compatibility (?? from the people who are changing the 
meaning of /
   ??)

Conclusion:

  o python upstream sucks hairy mammoths.

Bah!  Anyway, thanks for chasing this up Matthias, feel 
free to close
the bug, I guess I'll just have to work around upstream 
stupidity.


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

Comment By: Guido van Rossum (gvanrossum)
Date: 2001-04-10 13:33

Message:
Logged In: YES 
user_id=6380

I've looked into this, and I finally have an idea what's the
matter here.

The similarity with flock is intentional; the lockf and
flock calls are an abstraction level away from the fcntl
locking call.  By using the same arguments in both cases we
hoped to make things easier for the Python user.

We may not have succeeded there, but I prefer to document
the status quo over "fixing" the interface and breaking code
that uses it.

Therefore, I'm rejecting the patch.  The documentation has
been updated, see the working docs:

http://python.sourceforge.net/devel-docs/lib/module-fcntl.html


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

Comment By: Guido van Rossum (gvanrossum)
Date: 2001-01-18 19:37

Message:
I apologize. I should've looked into this earlier.  Now I won't have time to look at it before releasing 2.1a1.  I promise I'll come back to it later.

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

Comment By: A.M. Kuchling (akuchling)
Date: 2000-12-19 17:19

Message:
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.  


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

Comment By: Sjoerd Mullender (sjoerd)
Date: 2000-11-24 01:32

Message:
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.

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

Comment By: Guido van Rossum (gvanrossum)
Date: 2000-10-26 14:29

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

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

You can respond by visiting: 
http://sourceforge.net/tracker/?func=detail&atid=105470&aid=219486&group_id=5470