Re: Unexpected EINVAL from pthread_join

[Lasse Collin <lasse dot collin at tukaani dot org>]

On 2015-02-23 Corinna Vinschen wrote:
> On second thought, this is not the right way to handle this.  The
> WAIT_SIGNALED is returned because we're in the main thread and
> SA_RESTART is not set, as you assumed above.  This leads to the
> question why this scenario isn't handled directly in cygwait.
> 
> So what I did now is to apply the below patch to CVS.  It adds a flag
> cw_sig_restart to cygwait, which also restarts in the main thread if
> SA_RESTART is not set, as it's supposed to be for pthread_join.
> 
> I uploaded a new developer snapshot to https://cygwin.com/snapshots/
> Can you please test if it works as desired?

The snapshot 20150223 works. (The earlier patch worked too.) Thank you.

Many other pthread functions are similar in sense that they must never
return EINTR. A bug similar to the one in pthread::join exist in
pthread_mutex::lock. If SA_RESTART isn't used, signals can make
multiple threads get a lock on the same mutex at the same time. A test
program is attached. Adding cw_sig_restart to the cygwait call in
pthread_mutex::lock fixes this.

-- 
Lasse Collin  |  IRC: Larhzu @ IRCnet & Freenode

// gcc -std=gnu99 -Wall -Wextra -o mutex-test mutex-test.c

#include <pthread.h>
#include <signal.h>
#include <unistd.h>
#include <stdio.h>

#define dbg(expr) printf("%s = %d\n", #expr, expr)

static pthread_mutex_t mutex;

static void
sighandler(int sig)
{
	static const char msg[] = "Signal handler\n";
	write(STDOUT_FILENO, msg, sizeof(msg) - 1);
	return;
}

static void *
thr(void *ptr)
{
	printf("Thread started\n");
	dbg(pthread_mutex_lock(&mutex));
	sleep(5);
	printf("Thread exiting\n");
	return NULL;
}

extern int
main(void)
{
	// Setup a signal handler for SIGALRM. All signals will be
	// blocked while the signal handler executes. SA_RESTART
	// isn't used.
	sigset_t all;
	dbg(sigfillset(&all));

	struct sigaction act = {
		.sa_handler = &sighandler,
		.sa_mask = all,
		.sa_flags = 0,
	};

	dbg(sigaction(SIGALRM, &act, NULL));

	// Initialize the mutex.
	dbg(pthread_mutex_init(&mutex, NULL));

	// Create a new thread with all signals blocked.
	// This ensure that SIGALRM is sent to the main thread.
	pthread_t thread;
	sigset_t oldmask;
	dbg(pthread_sigmask(SIG_SETMASK, &all, &oldmask));
	dbg(pthread_create(&thread, NULL, &thr, NULL));
	dbg(pthread_sigmask(SIG_SETMASK, &oldmask, NULL));

	// Sleep to give the other thread time to lock the mutex.
	// Then locking the mutex in this thread will block.
	// SIGALRM should have no effect on pthread_mutex_lock()
	// and the program should deadlock here. This doesn't
	// happen if SA_RESTART isn't used and both threads appear
	// to get the lock.
	sleep(1);
	dbg(alarm(1));
	dbg(pthread_mutex_lock(&mutex));
	dbg(pthread_mutex_unlock(&mutex));

	sleep(5);

	return 0;
}

--
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