RE: Do PTHREAD_PROCESS_SHARED mutexes/condvars w ork?

The patch below against ngpt-2.0.4 that I provided to MontaVista should fix this 
issue. It comes from the changes made for ngpt-2.2.0 that will be released any day
now.

Dave Howell  

These are my opinions and not official opinions of Intel Corp.

David Howell
Intel Corporation
Telco Server Development
Server Products Division
Voice: (803) 461-6112  Fax: (803) 461-6292

Intel Corporation
Columbia Design Center, CBA-2
250 Berryhill Road, Suite 100
Columbia, SC 29210

[email protected]

-----

diff -Naur ngpt-2.0.4/ChangeLog ngpt-work.204/ChangeLog
--- ngpt-2.0.4/ChangeLog	Wed Nov 20 14:37:41 2002
+++ ngpt-work.204/ChangeLog	Wed Dec 11 19:12:09 2002
@@ -13,6 +13,13 @@
     / __/ _| |_| |
   _|_____(_)\___/_________________________________________________________
 
+  Changes post 2.0.4 from 2.2.0
+   *) pthread.c:
+   *) pth_sync.c:
+	Fixes from ngpt-2.2.0 to handle shared mutex hang cases.
+	(CGL bugzillas 1246).
+      [Dave Howell, <[email protected]>]
+
   Changes between 2.0.3 and 2.0.4
 
    *) ngpt.spec
diff -Naur ngpt-2.0.4/pth_sync.c ngpt-work.204/pth_sync.c
--- ngpt-2.0.4/pth_sync.c	Tue Nov 19 18:03:54 2002
+++ ngpt-work.204/pth_sync.c	Wed Dec 11 19:03:29 2002
@@ -408,7 +408,7 @@
 
 wait:
     /* already locked by caller? */
-    if (mutex->mx_owner == current && mutex->mx_count >= 1) {
+    if (mutex->mx_owner == current && mutex->mx_count >= 1 && mutex->mx_owner_pid==descr->pid) {
 	if (mutex->mx_type == PTH_MUTEX_RECURSIVE_NP) {
 	    /* recursive lock */
             mutex->mx_count++;
@@ -584,7 +584,6 @@
 	    current->mutex_owned = 0;
 	else
 	    pth_ring_delete(&(current->mutexring), &(mutex->mx_node));
-        _pth_release(&(mutex->mx_lock), descr->tid);
 	if (!mutex->mx_shared.pshared) {
 	    if (pth_number_of_natives > 1) {
 		mq = mutex->mx_waitlist.th_prev;
@@ -592,16 +591,41 @@
 		    char c = (int)1;
 		    pth_descr_t ds;
 		    tmp = (pth_t)((char *)mq - (int)&((pth_t)0)->mutex_cond_wait);
-		    if ((ds = tmp->boundnative))
+		    if ((ds = tmp->boundnative)) {
+			_pth_release(&(mutex->mx_lock), descr->tid);
 			pth_sc(write)(ds->sigpipe[1], &c, sizeof(char));
-		    else if (descr->current->boundnative)
+			return 0;
+		    } else {
+			ds = tmp->waited_native;
+			if (ds != NULL) {
+			    /* move waiting thread from wait-queue to ready-queue */
+			    _pth_acquire(&ds->wait_queue.q_lock, (pid_t) descr);
+			    if (tmp->waited_native) { /* to avoid race with eventmgr */
+				pth_pqueue_delete(&(ds->wait_queue), tmp);
+				tmp->waited_native = NULL;
+				_pth_release(&ds->wait_queue.q_lock, (pid_t) descr);
+
+				_pth_acquire(&pth_RQ.q_lock, (pid_t)descr);
+				tmp->state = PTH_STATE_READY;
+				pth_pqueue_insert(&pth_RQ, tmp->prio+1, tmp);
+				_pth_release(&pth_RQ.q_lock, (pid_t) descr);
+			    } else
+				_pth_release(&ds->wait_queue.q_lock, (pid_t) descr);
+			}
+			_pth_release(&(mutex->mx_lock), descr->tid);
+		    }
+		    if (descr->current->boundnative)
 			pth_wakeup_anative();
+		    return 0;
 		}
 	    }
-	} else
+	    _pth_release(&(mutex->mx_lock), descr->tid);
+	} else {
+	    _pth_release(&(mutex->mx_lock), descr->tid);
 	    errno_shield(
 		sys_futex_release(&(mutex->mx_shared));
 	    );
+	}
 	return 0;
     }
     _pth_release(&(mutex->mx_lock), descr->tid);
@@ -1038,13 +1062,13 @@
 		    }
 		    pth_sc(write)(ds->sigpipe[1], &c, sizeof(char));
 		    ds = NULL;
-		} else if (!broadcast) {
+		} else {
 		    ds = tmp->waited_native;
 		    if (ds != NULL) {
 			/* move waiting thread from wait-queue to ready-queue */
 			_pth_acquire_lock(&(ds->wait_queue.q_lock), descr->tid);
 			if (tmp->waited_native) { /* to avoid race with eventmgr */
-			    cond->cn_state |= PTH_COND_HANDLED;
+			    if (!broadcast) cond->cn_state |= PTH_COND_HANDLED;
 			    pth_pqueue_delete(&(ds->wait_queue), tmp);
 			    tmp->waited_native = NULL;
 			    _pth_release_lock(&(ds->wait_queue.q_lock), descr->tid);
@@ -1056,7 +1080,7 @@
 			} else
 			    _pth_release_lock(&(ds->wait_queue.q_lock), descr->tid);
                     }
-		    break;
+		    if (!broadcast) break;
 		}
 		cq = cq->th_prev;
 	    }
@@ -1087,13 +1111,13 @@
 		else if (ds != NULL && !ds->is_bound)
 		    pth_sc(write)(ds->sigpipe[1], &c, sizeof(char));
 	    } else if (cond->cn_waiters > 0) {
-		pth_wakeup_anative();
 	        if (!descr->current->boundnative) {
 		    /* wait until all waiters are awake... */
 		    ev = pth_event(PTH_EVENT_FUNC|PTH_MODE_STATIC, &ev_key, pth_check_waiters, 
 				   (void *)cond, pth_time_zero);
 		    pth_wait(ev);
 		} else {
+		    pth_wakeup_anative();
 		    while (cond->cn_waiters > 0)
 			pth_yield(NULL);
 		}
diff -Naur ngpt-2.0.4/pthread.c ngpt-work.204/pthread.c
--- ngpt-2.0.4/pthread.c	Tue Nov 19 18:03:54 2002
+++ ngpt-work.204/pthread.c	Wed Dec 11 18:58:20 2002
@@ -938,11 +938,12 @@
     pth_mutexattr_t *pattr = (attr != NULL) ? (pth_mutexattr_t *)(*attr) : NULL;
     pth_mutex_t *m;
 
-    if (mutex == NULL)
+    if (unlikely (mutex == NULL))
         return EINVAL;
-    if (pth_initialized_minimal != TRUE)
+    if (unlikely (pth_initialized_minimal != TRUE))
 	pth_initialize_minimal();
-    _pth_acquire_lock(&pth_init_lock, 0);
+    /* We use &mutex as lock owner, we don't want zeroes here :) */
+    _pth_acquire_lock(&pth_init_lock, (pid_t) &mutex);
     if (pth_initializing != TRUE && pth_shutdown_inprogress != TRUE) {
 	if (pth_init_lock.count > 1) { /* recursive call? */
 	    if (mutex_index < 100) 
@@ -956,18 +957,18 @@
 			/* if mutex is not consistent, make it consistent and return */
 			if (m->mx_state & PTH_MUTEX_NOT_CONSISTENT) {
 			    m->mx_state &= ~PTH_MUTEX_NOT_CONSISTENT;
-			    _pth_release_lock(&pth_init_lock, 0);
+			    _pth_release_lock(&pth_init_lock, (pid_t) &mutex);
 			    return OK;
 			}
 		    }
 		}
 		if ((m = (pth_mutex_t *)pth_alloc_shared_mutex()) == NULL) {
-		    _pth_release_lock(&pth_init_lock, 0);
+		    _pth_release_lock(&pth_init_lock, (pid_t) &mutex);
 		    return ENOMEM;
 		}
 	    } else {
 		if ((m = (pth_mutex_t *)pth_malloc(sizeof(pth_mutex_t))) == NULL) {
-		    _pth_release_lock(&pth_init_lock, 0);
+		    _pth_release_lock(&pth_init_lock, (pid_t) &mutex);
 		    return ENOMEM;
 		}
 	    }
@@ -975,7 +976,7 @@
     } else {
 	if (mutex_index < 100) {
 	    m = &init_mutex[mutex_index++];
-	    _pth_release_lock(&pth_init_lock, 0);
+	    _pth_release_lock(&pth_init_lock, (pid_t) &mutex);
 	    if (!pth_mutex_init(m, pattr)) 
 		return EINVAL;
 	    m->mx_state |= PTH_MUTEX_INTERNAL_LOCKED;
@@ -985,8 +986,8 @@
 	} else
 	    abort();
     }
-    _pth_release_lock(&pth_init_lock, 0);
-    if (!pth_mutex_init(m, pattr)) 
+    _pth_release_lock(&pth_init_lock, (pid_t) &mutex);
+    if (unlikely(!pth_mutex_init(m, pattr))) 
 	return EINVAL;
     mutex->mx_init = TRUE;
     mutex->mx_mutex = m;


-----Original Message-----
From: Howell, David P 
Sent: Tuesday, January 07, 2003 2:55 PM
To: Chris McFarlen; [email protected]
Subject: RE: [pthreads-devel] Do PTHREAD_PROCESS_SHARED mutexes/condvars w ork?

This change is already in the impending ngpt-2.2.0 that will be released very soon.
 
Dave Howell
 
-----Original Message-----
From: Chris McFarlen [mailto:[email protected]] 
Sent: Friday, December 27, 2002 4:14 PM
To: '[email protected]'
Subject: RE: [pthreads-devel] Do PTHREAD_PROCESS_SHARED mutexes/condvars w ork?
 
Well, I get to respond to my own question, but you guys can tell me if this is right.  This patch fixes the problem:
 
diff -Eu -x '*\.o' ngpt-2.0.4/pth_sync.c ngpt-2.0.4-my/pth_sync.c
--- ngpt-2.0.4/pth_sync.c 2002-11-19 12:03:54.000000000 -0600
+++ ngpt-2.0.4-my/pth_sync.c 2002-12-27 14:55:36.000000000 -0600
@@ -408,7 +408,9 @@
 
 wait:
     /* already locked by caller? */
-    if (mutex->mx_owner == current && mutex->mx_count >= 1) {
+    if (mutex->mx_owner == current && 
+        mutex->mx_owner_pid == descr->pid &&
+        mutex->mx_count >= 1) {
  if (mutex->mx_type == PTH_MUTEX_RECURSIVE_NP) {
      /* recursive lock */
             mutex->mx_count++;
Chris McFarlen
T-NETIX, Inc.
-----Original Message-----
From: Chris McFarlen 
Sent: Friday, December 27, 2002 2:58 PM
To: '[email protected]'
Subject: [pthreads-devel] Do PTHREAD_PROCESS_SHARED mutexes/condvars work?
I need to do some process syncronization, but the PTHREAD_PROCESS_SHARED attribute does not seem to work. I am using NGPT 2.0.4, and I have tried a stock 2.4.19 with the futex patch, and a 2.5.43 development kernel.  Both kernel versions have the same results.  I have some test code that yields the error, and I have made one modification to test_str03.c that also shows the error.  
The error comes from pthread_mutex_lock() when the mutex is locked by another process (the mutex data is shared via shared memory, and initialized with an attribute that has PTHREAD_PROCESS_SHARED set).  The error is EDEADLK.
I can post the test code I have and the change to get test_str03 to fail, if there is interest in helping out.  But I wanted to make sure that NGPT tries to support PTHREAD_PROCESS_SHARED before I posted all of that.  The documentation seems to say that NGPT does and does not support PTHREAD_PROCESS_SHARED in different places(there is a test for the case, test_str03, but you have to supply a -s parameter, and then that really doesn't work anyway...).
Thanks, 
Chris McFarlen 
T-NETIX, Inc.