Changing behaviour of pthread_cond_wait().

Changing behaviour of pthread_cond_wait().

[Takashi Yano]

[-- Attachment #1: Type: text/plain, Size: 1473 bytes --]

Hello,

I would like to propose chaging behaviour of pthread_cond_wait().

POSIX states as follows about pthread_cond_wait():
If a signal is delivered to a thread waiting for a condition variable,
upon return from the signal handler the thread resumes waiting for the
condition variable as if it was not interrupted, or it returns zero
due to spurious wakeup.

cygwin 2.7.0 employs the latter behaviour, while Debian GNU/Linux and
FreeBSD employ the former one. Of course, this is not a cygwin bug.
However, IMHO, it is better to have the compatibility with the other
major platforms.

Because of this difference, iperf 2.0.5 is not terminated normally in
cygwin 2.7.0. In client mode, iperf 2.0.5 does not stop after measurement
automatically. It needs ^C to stop it. In server mode, iperf 2.0.5 can
not stop even by ^C.  Simple test case, attached (pt.c), reproduces this
problem. I know this test case (and iperf 2.0.5) is essentially unsafe
because pthread functions are called from signal handler. But, anyway it
works in Debian GNU/Linux and FreeBSD.

The test case acts as follows in Debian GNU/Linux and FreeBSD.
% gcc pt.c -lpthread; ./a.out
Thread 1
Alarm 2
Thread 3
%

However, in cygwin, it acs as:
% gcc pt.c; ./a.exe
Thread 1
(Deadlock: ^C is needed to terminate.)
%

I would like to propose a patch attached (pthread.patch), for the above
reason. With this patch, iperf 2.0.5 as well as the test case works fine.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

[-- Attachment #2: pt.c --]
[-- Type: text/x-csrc, Size: 847 bytes --]

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

pthread_mutex_t m = PTHREAD_MUTEX_INITIALIZER;
pthread_cond_t c;
int flag = 0;

#define N 2

void sig_handler(int sig)
{
	pthread_mutex_lock(&m);
	flag ++;
	pthread_cond_signal(&c);
	printf("Alarm %d\n", flag); fflush(stdout);
	pthread_mutex_unlock(&m);
}

void *thread_main(void *args)
{
	int i;
	for (i=0; i<N; i++) {
		pthread_mutex_lock(&m);
		flag ++;
		pthread_cond_signal(&c);
		printf("Thread %d\n", flag); fflush(stdout);
		pthread_mutex_unlock(&m);
		usleep(2000000);
	}
	return NULL;
}

int main()
{
	pthread_t t;
	signal(SIGALRM, sig_handler);

	pthread_cond_init(&c, NULL);
	pthread_create(&t, NULL, thread_main, NULL);

	alarm(1);

	pthread_mutex_lock(&m);
	while (flag < N+1) {
		pthread_cond_wait(&c, &m);
	}
	pthread_mutex_unlock(&m);

	return 0;
}

[-- Attachment #3: pthread.patch --]
[-- Type: application/octet-stream, Size: 913 bytes --]

From d9c3c509d6b5606909d6419ee89ddd0421172614 Mon Sep 17 00:00:00 2001
From: Takashi Yano <takashi.yano@nifty.ne.jp>
Date: Tue, 7 Mar 2017 20:15:12 +0900
Subject: [PATCH] thread.cc: Now, pthread_cond::wait() does not return when a
 signal is delivered to the thread which called pthread_cond::wait(). This
 fixes the issue that iperf-2.0.5 is not terminated normally.

---
 winsup/cygwin/thread.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc
index 7084657..48c2efd 100644
--- a/winsup/cygwin/thread.cc
+++ b/winsup/cygwin/thread.cc
@@ -1266,7 +1266,7 @@ pthread_cond::wait (pthread_mutex_t mutex, PLARGE_INTEGER timeout)
   ++mutex->condwaits;
   mutex->unlock ();
 
-  rv = cygwait (sem_wait, timeout, cw_cancel | cw_sig_eintr);
+  rv = cygwait (sem_wait, timeout, cw_cancel | cw_sig | cw_sig_restart);
 
   mtx_out.lock ();
 
-- 
2.8.3


[-- Attachment #4: Type: text/plain, Size: 219 bytes --]


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

Re: Changing behaviour of pthread_cond_wait().

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1891 bytes --]

Hi,

On Mar  7 20:43, Takashi Yano wrote:
> Hello,
> 
> I would like to propose chaging behaviour of pthread_cond_wait().
> 
> POSIX states as follows about pthread_cond_wait():
> If a signal is delivered to a thread waiting for a condition variable,
> upon return from the signal handler the thread resumes waiting for the
> condition variable as if it was not interrupted, or it returns zero
> due to spurious wakeup.
> 
> cygwin 2.7.0 employs the latter behaviour, while Debian GNU/Linux and
> FreeBSD employ the former one. Of course, this is not a cygwin bug.
> However, IMHO, it is better to have the compatibility with the other
> major platforms.
> 
> Because of this difference, iperf 2.0.5 is not terminated normally in
> cygwin 2.7.0. In client mode, iperf 2.0.5 does not stop after measurement
> automatically. It needs ^C to stop it. In server mode, iperf 2.0.5 can
> not stop even by ^C.  Simple test case, attached (pt.c), reproduces this
> problem. I know this test case (and iperf 2.0.5) is essentially unsafe
> because pthread functions are called from signal handler. But, anyway it
> works in Debian GNU/Linux and FreeBSD.
> 
> The test case acts as follows in Debian GNU/Linux and FreeBSD.
> % gcc pt.c -lpthread; ./a.out
> Thread 1
> Alarm 2
> Thread 3
> %
> 
> However, in cygwin, it acs as:
> % gcc pt.c; ./a.exe
> Thread 1
> (Deadlock: ^C is needed to terminate.)
> %
> 
> I would like to propose a patch attached (pthread.patch), for the above
> reason. With this patch, iperf 2.0.5 as well as the test case works fine.

I applied a patch to change the behaviour as proposed, and uploaded
new snapshots to https://cygwin.com/snapshots/ for testing.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Changing behaviour of pthread_cond_wait().

[Takashi Yano]

On Tue, 7 Mar 2017 15:33:06 +0100 Corinna Vinschen wrote:

> I applied a patch to change the behaviour as proposed, and uploaded
> new snapshots to https://cygwin.com/snapshots/ for testing.

Confirmed. Thanks much.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

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