Re: flock still buggy

[Corinna Vinschen <corinna-cygwin at cygwin dot com>]

On Jul 22 12:08, Corinna Vinschen wrote:
> Does the below patch fix the problem?  Does the reasoning sound...
> reasonable?
> [...]
> -	node->del_my_locks (after_fork ? 0 : get_unique_id (), get_handle ());
> +	node->del_my_locks (after_fork ? 0 : get_unique_id (),
> +			    close_on_exec () ? NULL : get_handle ());
>        if (no_locks_left)
>  	{
>  	  LIST_REMOVE (node, i_next);

No, that can't be the right fix.  It might fix your problem, but it will
behave incorrectly if del_my_locks is called from close(), rather than
from fixup_after_exec().

Please try the below patch instead:

Index: fhandler.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler.h,v
retrieving revision 1.373
diff -u -p -r1.373 fhandler.h
--- fhandler.h	17 Jul 2009 22:51:28 -0000	1.373
+++ fhandler.h	22 Jul 2009 11:53:25 -0000
@@ -93,6 +93,12 @@ enum query_state {
   query_write_attributes = 4
 };
 
+enum del_lock_called_from {
+  after_close,
+  after_fork,
+  after_exec
+};
+
 class fhandler_base
 {
   friend class dtable;
@@ -141,7 +147,7 @@ class fhandler_base
 
   /* Used for advisory file locking.  See flock.cc.  */
   long long unique_id;
-  void del_my_locks (bool);
+  void del_my_locks (del_lock_called_from);
 
   HANDLE read_state;
   int wait_overlapped (bool, bool, DWORD *, DWORD = 0) __attribute__ ((regparm (3)));
Index: flock.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/flock.cc,v
retrieving revision 1.23
diff -u -p -r1.23 flock.cc
--- flock.cc	14 Jul 2009 17:37:42 -0000	1.23
+++ flock.cc	22 Jul 2009 11:53:26 -0000
@@ -344,14 +344,24 @@ inode_t::del_my_locks (long long id, HAN
    case the close_on_exec flag is set.  The whole inode is deleted as
    soon as no lock exists on it anymore. */
 void
-fhandler_base::del_my_locks (bool after_fork)
+fhandler_base::del_my_locks (del_lock_called_from from)
 {
   INODE_LIST_LOCK ();
   inode_t *node = inode_t::get (get_dev (), get_ino (), false);
   if (node)
     {
+      /* When we're called from fixup_after_exec, the fhandler is a
+	 close-on-exec fhandler.  In this case our io handle is already
+	 invalid.  We can't use it to test for the object reference count.
+	 However, that shouldn't be necessary for the following reason.
+	 After exec, there are no threads in the current process waiting for
+	 the lock.  So, either we're the only process accessing the file table
+	 entry and there are no threads which require signalling, or we have
+	 a parent process still accessing the file object and signalling the
+	 lock event would be premature. */
       bool no_locks_left =
-	node->del_my_locks (after_fork ? 0 : get_unique_id (), get_handle ());
+	node->del_my_locks (from == after_fork ? 0 : get_unique_id (),
+			    from == after_exec ? NULL : get_handle ());
       if (no_locks_left)
 	{
 	  LIST_REMOVE (node, i_next);


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple