* bsd_mutex.cc (_msleep): Simplify event creation. Revert change from

	2004-08-24.  It should be unnecessary now.
	* msg.cc (client_request_msg::serve): Release process critical
	section as early as possible.
	* sem.cc (client_request_sem::serve): Ditto.
	* shm.cc (client_request_shm::serve): Ditto.
	* process.cc: Use hold and release method calls instead of
	EnterCriticalSection/LeaveCriticalSection calls throughout.
	* process.h (_hold): Rename from hold.  Take filename and linenumber
	parameter for logging.  Define matching hold macro.
	(release): Ditto.

7 files changed, 43 insertions, 17 deletions

diff --git a/winsup/cygserver/ChangeLog b/winsup/cygserver/ChangeLog
index 5597350aa..225a1ebd0 100644
--- a/winsup/cygserver/ChangeLog
+++ b/winsup/cygserver/ChangeLog
@@ -1,3 +1,17 @@
+2004-10-04  Corinna Vinschen  <corinna@vinschen.de>
+
+	* bsd_mutex.cc (_msleep): Simplify event creation.  Revert change from
+	2004-08-24.  It should be unnecessary now.
+	* msg.cc (client_request_msg::serve): Release process critical
+	section as early as possible.
+	* sem.cc (client_request_sem::serve): Ditto.
+	* shm.cc (client_request_shm::serve): Ditto.
+	* process.cc: Use hold and release method calls instead of
+	EnterCriticalSection/LeaveCriticalSection calls throughout.
+	* process.h (_hold): Rename from hold.  Take filename and linenumber
+	parameter for logging.  Define matching hold macro.
+	(release): Ditto.
+
 2004-10-01  Corinna Vinschen  <corinna@vinschen.de>
 
 	* sysv_sem.cc: Update to FreeBSD version 1.69.
diff --git a/winsup/cygserver/bsd_mutex.cc b/winsup/cygserver/bsd_mutex.cc
index ad3c4a99d..873891458 100644
--- a/winsup/cygserver/bsd_mutex.cc
+++ b/winsup/cygserver/bsd_mutex.cc
@@ -180,9 +180,7 @@ _msleep (void *ident, struct mtx *mtx, int priority,
   int ret = -1;
   char name[64];
   msleep_event_name (ident, name);
-  HANDLE evt = OpenEvent (EVENT_ALL_ACCESS, FALSE, name);
-  if (!evt)
-    evt = CreateEvent (NULL, TRUE, FALSE, name);
+  HANDLE evt = CreateEvent (NULL, TRUE, FALSE, name);
   if (!evt)
     panic ("CreateEvent in msleep (%s) failed: %E", wmesg);
   if (mtx)
@@ -201,7 +199,6 @@ _msleep (void *ident, struct mtx *mtx, int priority,
   if ((priority & PCATCH)
       && td->client->signal_arrived () != INVALID_HANDLE_VALUE)
     obj_cnt = 4;
-  td->client->release ();
   switch (WaitForMultipleObjects (obj_cnt, obj, FALSE, timo ?: INFINITE))
     {
       case WAIT_OBJECT_0:	/* wakeup() has been called. */
@@ -233,7 +230,6 @@ _msleep (void *ident, struct mtx *mtx, int priority,
   ResetEvent (evt);
 #endif
   CloseHandle (evt);
-  td->client->hold ();
   set_priority (old_priority);
   if (mtx && !(priority & PDROP))
     mtx_lock (mtx);
diff --git a/winsup/cygserver/msg.cc b/winsup/cygserver/msg.cc
index 1e086ed06..87df87310 100644
--- a/winsup/cygserver/msg.cc
+++ b/winsup/cygserver/msg.cc
@@ -72,6 +72,7 @@ client_request_msg::serve (transport_layer_base *const conn,
     }
   if (!adjust_identity_info (&_parameters.in.ipcblk))
     {
+      client->release ();
       conn->revert_to_self ();
       error_code (EACCES);
       msglen (0);
@@ -79,6 +80,8 @@ client_request_msg::serve (transport_layer_base *const conn,
     }
   /* Early revert_to_self since IPC code runs in kernel mode. */
   conn->revert_to_self ();
+  /* sysv_msg.cc takes care of itself. */
+  client->release ();
   thread td = { client, &_parameters.in.ipcblk, {-1, -1} };
   int res;
   msgop_t msgop = _parameters.in.msgop; /* Get's overwritten otherwise. */
@@ -104,7 +107,6 @@ client_request_msg::serve (transport_layer_base *const conn,
   /* Allocated by the call to adjust_identity_info(). */
   if (_parameters.in.ipcblk.gidlist)
     free (_parameters.in.ipcblk.gidlist);
-  client->release ();
   error_code (res);
   if (msgop == MSGOP_msgrcv)
     _parameters.out.rcv = td.td_retval[0];
diff --git a/winsup/cygserver/process.cc b/winsup/cygserver/process.cc
index 94e7893a2..e03370d11 100644
--- a/winsup/cygserver/process.cc
+++ b/winsup/cygserver/process.cc
@@ -69,10 +69,12 @@ process::process (const pid_t cygpid, const DWORD winpid, HANDLE signal_arrived)
 		       GetLastError ());
     }
   InitializeCriticalSection (&_access);
+  debug ("initialized (%lu)", _cygpid);
 }
 
 process::~process ()
 {
+  debug ("deleting (%lu)", _cygpid);
   DeleteCriticalSection (&_access);
   CloseHandle (_signal_arrived);
   CloseHandle (_hProcess);
@@ -105,7 +107,7 @@ process::add (cleanup_routine *const entry)
   assert (entry);
 
   bool res = false;
-  EnterCriticalSection (&_access);
+  hold ();
 
   if (!_cleaning_up)
     {
@@ -114,7 +116,7 @@ process::add (cleanup_routine *const entry)
       res = true;
     }
 
-  LeaveCriticalSection (&_access);
+  release ();
   return res;
 }
 
@@ -124,7 +126,7 @@ process::remove (const cleanup_routine *const entry)
   assert (entry);
 
   bool res = false;
-  EnterCriticalSection (&_access);
+  hold ();
 
   if (!_cleaning_up)
     {
@@ -148,7 +150,7 @@ process::remove (const cleanup_routine *const entry)
 	}
     }
 
-  LeaveCriticalSection (&_access);
+  release ();
   return res;
 }
 
@@ -159,13 +161,13 @@ process::remove (const cleanup_routine *const entry)
 void
 process::cleanup ()
 {
-  EnterCriticalSection (&_access);
+  hold ();
   assert (!is_active ());
   assert (!_cleaning_up);
   InterlockedExchange (&_cleaning_up, true);
   cleanup_routine *entry = _routines_head;
   _routines_head = NULL;
-  LeaveCriticalSection (&_access);
+  release ();
 
   while (entry)
     {
@@ -275,7 +277,7 @@ process_cache::process (const pid_t cygpid, const DWORD winpid,
       SetEvent (_cache_add_trigger);
     }
 
-  EnterCriticalSection (&entry->_access); // To be released by the caller.
+  entry->hold (); // To be released by the caller.
   LeaveCriticalSection (&_cache_write_access);
   assert (entry);
   assert (entry->_winpid == winpid);
diff --git a/winsup/cygserver/process.h b/winsup/cygserver/process.h
index 75d0c72d3..16a5205e9 100644
--- a/winsup/cygserver/process.h
+++ b/winsup/cygserver/process.h
@@ -65,6 +65,9 @@ private:
 
 class process_cache;
 
+#define hold()		_hold(__FILE__,__LINE__)
+#define release()	_release(__FILE__,__LINE__)
+
 class process
 {
   friend class process_cache;
@@ -82,8 +85,15 @@ public:
 
   bool is_active () const { return _exit_status == STILL_ACTIVE; }
 
-  void hold () { EnterCriticalSection (&_access); }
-  void release () { LeaveCriticalSection (&_access); }
+  void _hold (const char *file, int line) {
+    _log (file, line, LOG_DEBUG, "Try hold(%lu)", _cygpid);
+    EnterCriticalSection (&_access);
+    _log (file, line, LOG_DEBUG, "holding (%lu)", _cygpid);
+  }
+  void _release (const char *file, int line) {
+    _log (file, line, LOG_DEBUG, "leaving (%lu)", _cygpid);
+    LeaveCriticalSection (&_access);
+  }
 
   bool add (cleanup_routine *);
   bool remove (const cleanup_routine *);
diff --git a/winsup/cygserver/sem.cc b/winsup/cygserver/sem.cc
index f0fbbac0b..37d894ef9 100644
--- a/winsup/cygserver/sem.cc
+++ b/winsup/cygserver/sem.cc
@@ -77,6 +77,8 @@ client_request_sem::serve (transport_layer_base *const conn,
     }
   /* Early revert_to_self since IPC code runs in kernel mode. */
   conn->revert_to_self ();
+  /* sysv_sem.cc takes care of itself. */
+  client->release ();
   thread td = { client, &_parameters.in.ipcblk, {-1, -1} };
   int res;
   switch (_parameters.in.semop)
@@ -98,7 +100,6 @@ client_request_sem::serve (transport_layer_base *const conn,
   /* Allocated by the call to adjust_identity_info(). */
   if (_parameters.in.ipcblk.gidlist)
     free (_parameters.in.ipcblk.gidlist);
-  client->release ();
   error_code (res);
   _parameters.out.ret = td.td_retval[0];
   msglen (sizeof (_parameters.out));
diff --git a/winsup/cygserver/shm.cc b/winsup/cygserver/shm.cc
index 17b05bfdd..38f2d2ee5 100644
--- a/winsup/cygserver/shm.cc
+++ b/winsup/cygserver/shm.cc
@@ -80,6 +80,8 @@ client_request_shm::serve (transport_layer_base *const conn,
     }
   /* Early revert_to_self since IPC code runs in kernel mode. */
   conn->revert_to_self ();
+  /* sysv_shm.cc takes care of itself. */
+  client->release ();
   thread td = { client, &_parameters.in.ipcblk, {0, 0} };
   int res;
   shmop_t shmop = _parameters.in.shmop; /* Get's overwritten otherwise. */
@@ -110,7 +112,6 @@ client_request_shm::serve (transport_layer_base *const conn,
   /* Allocated by the call to adjust_identity_info(). */
   if (_parameters.in.ipcblk.gidlist)
     free (_parameters.in.ipcblk.gidlist);
-  client->release ();
   error_code (res);
   if (shmop == SHMOP_shmat)
     _parameters.out.ptr = td.td_retval[0];