Skip to contentSkip to conversation

POSIX: Allow pthread_barrier_destroy() to block

PATCH New Richard Palethorpe 2 participants 3 messages Nov 1, 2017

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/2 POSIX: Allow pthread_barrier_destroy() to block

Newer glibc versions choose to block until a barrier's users have all synchronised with it. This caused the test to hang forever, so a watchdog has been introduced to complete the barrier after a timeout.

---
 .../interfaces/pthread_barrier_destroy/2-1.c       | 105 +++++++++++++--------
 1 file changed, 64 insertions(+), 41 deletions(-)
testcases/open_posix_testsuite/conformance/interfaces/pthread_barrier_destroy/2-1.c +64 -41
index 7abb08ff8..fb782f2f3 100644
@@ -1,23 +1,25 @@
 /*
  * Copyright (c) 2002, Intel Corporation. All rights reserved.
+ * Copyright (c) 2017, Richard Palethorpe <[email protected]>
  * This file is licensed under the GPL license.  For the full content
  * of this license, see the COPYING file at the top level of this
  * source tree.
  *
  * pthread_barrier_destroy()
  *
- * The pthread_barrier_destroy() function may fail if:
- * [EBUSY] The implementation has detected an attempt to destroy a barrier while it is in
- * use (for example, while being used in a pthread_barrier_wait() call) by another
- * thread.
+ * Attempt to destroy a barrier which has a waiter. The standard recommends
+ * that EBUSY is returned, but the required behaviour is undefined. At the
+ * time of writing, glibc blocks until there are no threads waiting on the
+ * barrier.
  *
- * Note: This case will always PASS
+ * Hypothetically, on some implementations, the child could segfault or
+ * 'barrier_wait() could return success or EINVAL when 'barrier_destroy() is
+ * called. It is difficult to determine whether 'barrier_wait() has returned
+ * due to 'barrier_destroy() or some other reason. So we just report what has
+ * happened. Obviously a segfault will cause the test to crash, which is
+ * highly undesirable behaviour regardless of whether it is POSIX compliant.
  *
- * Steps:
- * 1. Main initialize barrier with count 2
- * 2. Main create a child thread
- * 3. Child thread call pthread_barrier_wait(), should block
- * 4. Main call pthread_barrier_destroy(), while child is blocking on the barrier
+ * Also see pthread_cond_destroy 4-1.
  */
 
 #define _XOPEN_SOURCE 600
@@ -28,13 +30,15 @@
 #include <signal.h>
 #include <string.h>
 #include <errno.h>
+#include <sched.h>
 #include "posixtest.h"
 
 static pthread_barrier_t barrier;
-static int thread_state;
+static volatile int thread_state;
 #define NOT_CREATED_THREAD 1
 #define ENTERED_THREAD 2
 #define EXITING_THREAD 3
+#define WAIT_LOOP 0xFFFFFF
 
 static void *fn_chld(void *arg)
 {
@@ -42,74 +46,93 @@ static void *fn_chld(void *arg)
 
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
 
+	printf("child: barrier wait\n");
 	thread_state = ENTERED_THREAD;
+	rc = pthread_barrier_wait(&barrier);
+	if (rc == PTHREAD_BARRIER_SERIAL_THREAD)
+		printf("child: got PTHREAD_BARRIER_SERIAL_THREAD\n");
+	else if (rc != 0)
+		perror("child: pthread_barrier_wait");
+	else
+		printf("child: pthread_barrier_wait returned success");
 
-	/* Child should block here */
-	printf("child: barrier wait\n");
+	thread_state = EXITING_THREAD;
+	return arg;
+}
+
+static void *watchdog(void *arg)
+{
+	int rc = 0;
+
+	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
+
+	sleep(1);
+	printf("watchdog: It appears pthread_barrier_destroy() is blocking\n");
 	rc = pthread_barrier_wait(&barrier);
 	if (rc != 0 && rc != PTHREAD_BARRIER_SERIAL_THREAD) {
-		printf("Error: child: pthread_barrier_wait() get unexpected "
-		       "return code : %d\n", rc);
+		perror("watchdog: pthread_barrier_wait");
 		exit(PTS_UNRESOLVED);
-	} else if (rc == PTHREAD_BARRIER_SERIAL_THREAD) {
-		printf("child: get PTHREAD_BARRIER_SERIAL_THREAD\n");
 	}
 
-	thread_state = EXITING_THREAD;
-	pthread_exit(0);
-	return NULL;
+	return arg;
 }
 
 int main(void)
 {
 	int cnt = 0;
 	int rc;
-	pthread_t child_thread;
+	pthread_t child_thread, watchdog_thread;
 
 	printf("main: Initialize barrier with count = 2\n");
 	if (pthread_barrier_init(&barrier, NULL, 2) != 0) {
-		printf("main: Error at pthread_barrier_init()\n");
+		perror("main: pthread_barrier_init");
 		return PTS_UNRESOLVED;
 	}
 
 	printf("main: create child thread\n");
 	thread_state = NOT_CREATED_THREAD;
 	if (pthread_create(&child_thread, NULL, fn_chld, NULL) != 0) {
-		printf("main: Error at pthread_create()\n");
+		perror("main: pthread_create");
+		return PTS_UNRESOLVED;
+	}
+
+	printf("main: create watchdog thread\n");
+	if (pthread_create(&watchdog_thread, NULL, watchdog, NULL) != 0) {
+		perror("main: Error at pthread_create");
 		return PTS_UNRESOLVED;
 	}
 
-	/* Expect the child to block */
 	cnt = 0;
-	do {
-		sleep(1);
-	} while (thread_state != EXITING_THREAD && cnt++ < 2);
+	for (cnt = 0; thread_state < ENTERED_THREAD && cnt < WAIT_LOOP; cnt++)
+		sched_yield();
+	/* Yield once more to increase the probability that the child thread
+	 * will call pthread_barrier_wait() before this thread reaches
+	 * pthread_barrier_destroy().
+	 */
+	sched_yield();
 
 	if (thread_state == EXITING_THREAD) {
-		/* child thread did not block */
-		printf("Test FAILED: child thread did not block on "
-		       "pthread_barrier_wait()\n");
+		printf("Test FAILED: child thread did not block on pthread_barrier_wait()\n");
 		exit(PTS_FAIL);
-	} else if (thread_state != ENTERED_THREAD) {
-		printf("Unexpected thread state\n");
+	} else if (thread_state == NOT_CREATED_THREAD) {
+		printf("Child thread did not start (quick enough)\n");
 		exit(PTS_UNRESOLVED);
 	}
 
 	printf("main: destroy barrier while child is waiting\n");
-
 	rc = pthread_barrier_destroy(&barrier);
 
-	if (rc == EBUSY) {
-		printf("main: correctly got EBUSY\n");
-		printf("Test PASSED\n");
+	if (rc != EBUSY) {
+		printf("UNSUPPORTED: POSIX recommends returning EBUSY, but got: %d, %s\n",
+		       rc, strerror(rc));
+		rc = PTS_UNSUPPORTED;
 	} else {
-		printf("main: got return code: %d, %s\n", rc, strerror(rc));
-		printf
-		    ("Test PASSED: Note*: Expected EBUSY, but standard says 'may' fail.\n");
+		printf("Test PASSED\n");
+		rc = PTS_PASS;
 	}
 
-	/* Cleanup (cancel thread in case it is still blocking */
+	pthread_cancel(watchdog_thread);
 	pthread_cancel(child_thread);
 
-	return PTS_PASS;
+	return rc;
 }
Signature · 1 line
2.14.3
#1 lore raw

PATCH v2 2/2 POSIX: Allow pthread_cond_destroy to block

---
 .../pthread_cond_destroy/speculative/4-1.c         | 86 +++++++++++++---------
 1 file changed, 50 insertions(+), 36 deletions(-)
testcases/open_posix_testsuite/conformance/interfaces/pthread_cond_destroy/speculative/4-1.c +50 -36
index 9df7c1993..b447a2ffe 100644
@@ -1,19 +1,17 @@
 /*
  * Copyright (c) 2004, QUALCOMM Inc. All rights reserved.
+ * Copyright (c) 2017, Richard Palethorpe <[email protected]>
  * Created by:  abisain REMOVE-THIS AT qualcomm DOT com
  * This file is licensed under the GPL license.  For the full content
  * of this license, see the COPYING file at the top level of this
  * source tree.
-
- * Test that when  pthread_cond_destroy()
- *   is called on a cond that some thread is waiting, then it returns
- *   EBUSY
-
- * Steps:
- * 1. Create a condvar
- * 2. Create a thread and make it wait on the condvar
- * 3. Try to destroy the cond var in main
- * 4. Check that pthread_cond_destroy returns EBUSY
+ *
+ * Test that EBUSY is returned when pthread_cond_destroy() is called on a cond
+ * var that has waiters. POSIX only recommends this behaviour, the required
+ * behaviour is undefined.
+ *
+ * This test is very similar to pthread_barrier_destroy 2-1 which has more
+ * explanation attached.
  */
 
 #include <pthread.h>
@@ -21,68 +19,84 @@
 #include <stdlib.h>
 #include <errno.h>
 #include <unistd.h>
+#include <string.h>
 #include "posixtest.h"
 
-#define TEST "4-1"
-#define FUNCTION "pthread_cond_destroy"
-#define ERROR_PREFIX "unexpected error: " FUNCTION " " TEST ": "
-
-/* cond used by the two threads */
 pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
-
-/* cond used by the two threads */
 pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
 
 void *thread(void *tmp)
 {
 	int rc = 0;
 
-	/* acquire the mutex */
+	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
+
 	rc = pthread_mutex_lock(&mutex);
 	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_mutex_lock\n");
+		perror("child: pthread_mutex_lock");
 		exit(PTS_UNRESOLVED);
 	}
 
-	/* Wait on the cond var. This will not return, as nobody signals */
 	rc = pthread_cond_wait(&cond, &mutex);
 	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_cond_wait\n");
-		exit(PTS_UNRESOLVED);
+		perror("child: pthread_cond_wait");
+		exit(PTS_UNSUPPORTED);
 	}
 
 	rc = pthread_mutex_unlock(&mutex);
 	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_mutex_unlock\n");
+		perror("child: pthread_mutex_unlock");
+		exit(PTS_UNSUPPORTED);
+	}
+
+	return tmp;
+}
+
+void *watchdog(void *arg)
+{
+	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
+
+	sleep(1);
+	printf("watchdog: pthread_cond_destroy() appears to be blocking\n");
+	if (pthread_cond_signal(&cond)) {
+		perror("watchdog: pthread_cond_signal()");
 		exit(PTS_UNRESOLVED);
 	}
-	return NULL;
+
+	return arg;
 }
 
 int main(void)
 {
-	pthread_t low_id;
+	pthread_t low_id, watchdog_thread;
 	int rc = 0;
 
-	/* Create a new thread with default attributes */
 	rc = pthread_create(&low_id, NULL, thread, NULL);
 	if (rc != 0) {
-		printf(ERROR_PREFIX "pthread_create\n");
+		perror("main: pthread_create");
 		exit(PTS_UNRESOLVED);
 	}
 
-	/* Let the other thread run */
-	sleep(2);
+	sleep(1);
+
+	rc = pthread_create(&watchdog_thread, NULL, watchdog, NULL);
+	if (rc != 0) {
+		perror("main: pthread_create");
+		exit(PTS_UNRESOLVED);
+	}
 
-	/* Try to destroy the cond var. This should return an error */
 	rc = pthread_cond_destroy(&cond);
 	if (rc != EBUSY) {
-		printf(ERROR_PREFIX "Test PASS: Expected %d(EBUSY) got %d, "
-		       "though the standard states 'may' fail\n", EBUSY, rc);
-
-		exit(PTS_PASS);
+		printf("UNSUPPORTED: The standard recommends returning %d, EBUSY, but got %d, %s\n",
+		       EBUSY, rc, strerror(rc));
+		rc = PTS_UNSUPPORTED;
+	} else {
+		printf("PASSED: received EBUSY as per recommendation\n");
+		rc = PTS_PASS;
 	}
 
-	printf("Test PASSED\n");
-	exit(PTS_PASS);
+	pthread_cancel(watchdog_thread);
+	pthread_cancel(low_id);
+
+	exit(rc);
 }
Signature · 1 line
2.14.3