* exceptions.cc (try_to_debug): Report on tid of caller.

* sync.cc (muto::acquire): Fix some races.
* sync.h (muto): Expose some fields for easier debugging.

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index a6eccbd..d47ddff 100644 (file)
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,10 @@
+2004-03-15  Christopher Faylor  <cgf@redhat.com>
+
+       * exceptions.cc (try_to_debug): Report on tid of caller.
+
+       * sync.cc (muto::acquire): Fix some races.
+       * sync.h (muto): Expose some fields for easier debugging.
+
 2004-03-15  Corinna Vinschen  <corinna@vinschen.de>
 
        * fhandler.h (fhandler_dev_tape::tape_get_pos): Declare with extra

diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 5ad6642..3b7be70 100644 (file)
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -349,8 +349,8 @@ try_to_debug (bool waitloop)
        }
     }
 
-  console_printf ("*** starting debugger for pid %u\n",
-                 cygwin_pid (GetCurrentProcessId ()));
+  console_printf ("*** starting debugger for pid %u, tid %u\n",
+                 cygwin_pid (GetCurrentProcessId ()), GetCurrentThreadId ());
   BOOL dbg;
   dbg = CreateProcess (NULL,
                       debugger_command,

diff --git a/winsup/cygwin/sync.cc b/winsup/cygwin/sync.cc
index f65e5a8..bd89793 100644 (file)
--- a/winsup/cygwin/sync.cc
+++ b/winsup/cygwin/sync.cc
@@ -64,7 +64,8 @@ muto::~muto ()
 
    Note: The goal here is to minimize, as much as possible, calls to the
    OS.  Hence the use of InterlockedIncrement, etc., rather than (much) more
-   expensive OS mutexes.  */
+   expensive OS mutexes.  Also note that the only two valid "ms" times are
+   0 and INFINITE. */
 int
 muto::acquire (DWORD ms)
 {
@@ -78,35 +79,27 @@ muto::acquire (DWORD ms)
     {
       /* Increment the waiters part of the class.  Need to do this first to
         avoid potential races. */
-      LONG was_waiting = InterlockedIncrement (&waiters);
-
-      /* This is deceptively simple.  Basically, it allows multiple attempts to
-        lock the same muto to succeed without attempting to manipulate sync.
-        If the muto is already locked then this thread will wait for ms until
-        it is signalled by muto::release.  Then it will attempt to grab the
-        sync field.  If it succeeds, then this thread owns the muto.
-
-        There is a pathological condition where a thread times out waiting for
-        bruteforce but the release code triggers the bruteforce event.  In this
-        case, it is possible for a thread which is going to wait for bruteforce
-        to wake up immediately.  It will then attempt to grab sync but will fail
-        and go back to waiting.  */
-      if (tid != this_tid && (was_waiting || InterlockedExchange (&sync, 1) != 0))
-       {
-         switch (WaitForSingleObject (bruteforce, ms))
-             {
-             case WAIT_OBJECT_0:
-               goto gotit;
-               break;
-             default:
-               InterlockedDecrement (&waiters);
-               return 0;       /* failed. */
-             }
-       }
+      LONG was_waiting = ms ? InterlockedIncrement (&waiters) : 0;
+
+      while (was_waiting || InterlockedExchange (&sync, 1) != 0)
+         {
+           switch (WaitForSingleObject (bruteforce, ms))
+               {
+               case WAIT_OBJECT_0:
+                 was_waiting = 0;
+                 break;
+               default:
+                 return 0;     /* failed. */
+               }
+         }
+
+      /* Have to do it this way to avoid a race */
+      if (!ms)
+       InterlockedIncrement (&waiters);
+
+      tid = this_tid;  /* register this thread. */
     }
 
-gotit:
-  tid = this_tid;      /* register this thread. */
   return ++visits;     /* Increment visit count. */
 }
 
@@ -129,7 +122,7 @@ muto::release ()
       (void) InterlockedExchange (&sync, 0); /* Reset trigger. */
       /* This thread had incremented waiters but had never decremented it.
         Decrement it now.  If it is >= 0 then there are possibly other
-        threads waiting for the lock, so trigger bruteforce. */
+        threads waiting for the lock, so trigger bruteforce.  */
       if (InterlockedDecrement (&waiters) >= 0)
        (void) SetEvent (bruteforce); /* Wake up one of the waiting threads */
     }

diff --git a/winsup/cygwin/sync.h b/winsup/cygwin/sync.h
index 75b0b16..24de974 100644 (file)
--- a/winsup/cygwin/sync.h
+++ b/winsup/cygwin/sync.h
@@ -18,11 +18,11 @@ class muto
 {
   static DWORD exiting_thread;
   LONG sync;   /* Used to serialize access to this class. */
-  LONG visits; /* Count of number of times a thread has called acquire. */
   LONG waiters;        /* Number of threads waiting for lock. */
   HANDLE bruteforce; /* event handle used to control waiting for lock. */
-  DWORD tid;   /* Thread Id of lock owner. */
 public:
+  LONG visits; /* Count of number of times a thread has called acquire. */
+  DWORD tid;   /* Thread Id of lock owner. */
   // class muto *next;
   const char *name;