Bug 37658

Summary:	libc_r: poll(2) does not wake up when thread canceled
Product:	Base System	Reporter:	Archie Cobbs <archie>
Component:	bin	Assignee:	Archie Cobbs <archie>
Status:	Closed FIXED
Severity:	Affects Only Me
Priority:	Normal
Version:	4.5-RELEASE
Hardware:	Any
OS:	Any

Description Archie Cobbs 2002-05-02 02:40:01 UTC

	Have a thread block in poll(2), which is a cancellation point.
	Have another thread cancel that thread with pthread_cancel().

	Problem: the cancellation does not take effect until the
	poll(2) returns, which can be arbitrarily long in the future
	or even never!

Fix: 

Looks like the PTHREAD_AT_CANCEL_POINT flag needs to be set for the
	current thread in the libc_r version of poll(2).

	NOTE: there may be other system calls with this same problem.
	Perhaps a quick audit should be done.
How-To-Repeat: 
Run this program - it should exit in 2 seconds, but it actually takes 4

---------------------------------------------------------------------------

#include <stdio.h>
#include <signal.h>
#include <unistd.h>
#include <errno.h>
#include <sched.h>
#include <pthread.h>
#include <poll.h>
#include <err.h>

#define MAIN_WAIT	2
#define THREAD_WAIT	4

static void
thread_final(void *arg)
{
	printf("Thread: exiting...\n");
}

static void *
thread_main(void *arg)
{
	struct pollfd pfd;

	pthread_cleanup_push(thread_final, NULL);
	memset(&pfd, 0, sizeof(pfd));
	pfd.fd = 0;
	pfd.events = POLLERR;
	printf("Thread: polling for %d seconds...\n", THREAD_WAIT);
	if (poll(&pfd, 1, THREAD_WAIT * 1000) == -1)
		err(1, "poll");
	return (NULL);
}

int
main(int argc, char **argv)
{
	pthread_t tid;

	printf("
Demonstration of bug in poll(2) where canceling a thread
doesn't immediately wake it up from a poll(2).

");

	/* Spawn thread */
	printf("Main: spawning thread...\n");
	if ((errno = pthread_create(&tid, NULL, thread_main, NULL)) != 0)
		err(1, "pthread_create");

	/* Pause */
	printf("Main: sleeping %d seconds...\n", MAIN_WAIT);
	sleep(MAIN_WAIT);

	/* Cancel thread */
	printf("Main: canceling thread...\n");
	pthread_cancel(tid);
	printf("Main: joining thread...\n");
	pthread_join(tid, NULL);

	/* Done */
	printf("Main: exiting...\n");
	return (0);
}

---------------------------------------------------------------------------

Comment 1 Archie Cobbs 2002-05-02 19:38:59 UTC

Below is a patch that fixes the problem for me. It also fixes
the same problem for these functions: select, readv, wait4, writev.

-Archie

__________________________________________________________________________
Archie Cobbs     *     Packet Design     *     http://www.packetdesign.com


Index: uthread_poll.c
===================================================================
RCS file: /home/cvs/freebsd/src/lib/libc_r/uthread/uthread_poll.c,v
retrieving revision 1.9
diff -u -r1.9 uthread_poll.c
--- uthread_poll.c	29 Jan 2000 22:53:49 -0000	1.9
+++ uthread_poll.c	2 May 2002 18:38:30 -0000
@@ -97,5 +97,15 @@
 	return (ret);
 }
 
-__strong_reference(_poll, poll);
+int
+poll(struct pollfd *fds, unsigned int nfds, int timeout)
+{
+	int ret;
+
+	_thread_enter_cancellation_point();
+	ret = _poll(fds, nfds, timeout);
+	_thread_leave_cancellation_point();
+
+	return ret;
+}
 #endif
Index: uthread_readv.c
===================================================================
RCS file: /home/cvs/freebsd/src/lib/libc_r/uthread/uthread_readv.c,v
retrieving revision 1.11
diff -u -r1.11 uthread_readv.c
--- uthread_readv.c	29 Jan 2000 22:53:49 -0000	1.11
+++ uthread_readv.c	2 May 2002 18:38:30 -0000
@@ -91,5 +91,15 @@
 	return (ret);
 }
 
-__strong_reference(_readv, readv);
+ssize_t
+readv(int fd, const struct iovec *iov, int iovcnt)
+{
+	ssize_t ret;
+
+	_thread_enter_cancellation_point();
+	ret = _readv(fd, iov, iovcnt);
+	_thread_leave_cancellation_point();
+
+	return ret;
+}
 #endif
Index: uthread_select.c
===================================================================
RCS file: /home/cvs/freebsd/src/lib/libc_r/uthread/uthread_select.c,v
retrieving revision 1.16
diff -u -r1.16 uthread_select.c
--- uthread_select.c	29 Jan 2000 22:53:50 -0000	1.16
+++ uthread_select.c	2 May 2002 18:38:30 -0000
@@ -204,5 +204,16 @@
 	return (ret);
 }
 
-__strong_reference(_select, select);
+int 
+select(int numfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
+	struct timeval *timeout)
+{
+	int ret;
+
+	_thread_enter_cancellation_point();
+	ret = _select(numfds, readfds, writefds, exceptfds, timeout);
+	_thread_leave_cancellation_point();
+
+	return ret;
+}
 #endif
Index: uthread_wait4.c
===================================================================
RCS file: /home/cvs/freebsd/src/lib/libc_r/uthread/uthread_wait4.c,v
retrieving revision 1.10.2.1
diff -u -r1.10.2.1 uthread_wait4.c
--- uthread_wait4.c	9 Nov 2000 23:46:04 -0000	1.10.2.1
+++ uthread_wait4.c	2 May 2002 18:38:30 -0000
@@ -67,5 +67,15 @@
 	return (ret);
 }
 
-__strong_reference(_wait4, wait4);
+pid_t
+wait4(pid_t pid, int *istat, int options, struct rusage *rusage)
+{
+	pid_t ret;
+
+	_thread_enter_cancellation_point();
+	ret = _wait4(pid, istat, options, rusage);
+	_thread_leave_cancellation_point();
+
+	return ret;
+}
 #endif
Index: uthread_writev.c
===================================================================
RCS file: /home/cvs/freebsd/src/lib/libc_r/uthread/uthread_writev.c,v
retrieving revision 1.16
diff -u -r1.16 uthread_writev.c
--- uthread_writev.c	29 Jan 2000 22:53:55 -0000	1.16
+++ uthread_writev.c	2 May 2002 18:38:31 -0000
@@ -201,5 +201,15 @@
 	return (ret);
 }
 
-__strong_reference(_writev, writev);
+ssize_t
+writev(int fd, const struct iovec *iov, int iovcnt)
+{
+	ssize_t ret;
+
+	_thread_enter_cancellation_point();
+	ret = _writev(fd, iov, iovcnt);
+	_thread_leave_cancellation_point();
+
+	return ret;
+}
 #endif

Comment 2 Archie Cobbs 2002-05-02 20:03:00 UTC

Here is the corresponding patch for -current.

-Archie

__________________________________________________________________________
Archie Cobbs     *     Packet Design     *     http://www.packetdesign.com

Index: uthread_poll.c
===================================================================
RCS file: /home/ncvs/src/lib/libc_r/uthread/uthread_poll.c,v
retrieving revision 1.11
diff -u -r1.11 uthread_poll.c
--- uthread_poll.c	10 Apr 2001 04:19:20 -0000	1.11
+++ uthread_poll.c	2 May 2002 18:58:14 -0000
@@ -41,7 +41,7 @@
 #include <pthread.h>
 #include "pthread_private.h"
 
-__weak_reference(_poll, poll);
+__weak_reference(__poll, poll);
 
 int 
 _poll(struct pollfd *fds, unsigned int nfds, int timeout)
@@ -96,4 +96,16 @@
 	}
 
 	return (ret);
+}
+
+int
+__poll(struct pollfd *fds, unsigned int nfds, int timeout)
+{
+	int ret;
+
+	_thread_enter_cancellation_point();
+	ret = _poll(fds, nfds, timeout);
+	_thread_leave_cancellation_point();
+
+	return ret;
 }
Index: uthread_readv.c
===================================================================
RCS file: /home/ncvs/src/lib/libc_r/uthread/uthread_readv.c,v
retrieving revision 1.13
diff -u -r1.13 uthread_readv.c
--- uthread_readv.c	10 Apr 2001 04:19:20 -0000	1.13
+++ uthread_readv.c	2 May 2002 18:58:14 -0000
@@ -40,7 +40,7 @@
 #include <pthread.h>
 #include "pthread_private.h"
 
-__weak_reference(_readv, readv);
+__weak_reference(__readv, readv);
 
 ssize_t
 _readv(int fd, const struct iovec * iov, int iovcnt)
@@ -91,4 +91,16 @@
 		_FD_UNLOCK(fd, FD_READ);
 	}
 	return (ret);
+}
+
+ssize_t
+__readv(int fd, const struct iovec *iov, int iovcnt)
+{
+	ssize_t ret;
+
+	_thread_enter_cancellation_point();
+	ret = _readv(fd, iov, iovcnt);
+	_thread_leave_cancellation_point();
+
+	return ret;
 }
Index: uthread_select.c
===================================================================
RCS file: /home/ncvs/src/lib/libc_r/uthread/uthread_select.c,v
retrieving revision 1.19
diff -u -r1.19 uthread_select.c
--- uthread_select.c	9 Apr 2002 05:41:00 -0000	1.19
+++ uthread_select.c	2 May 2002 18:58:14 -0000
@@ -43,7 +43,7 @@
 #include <pthread.h>
 #include "pthread_private.h"
 
-__weak_reference(_select, select);
+__weak_reference(__select, select);
 
 int 
 _select(int numfds, fd_set * readfds, fd_set * writefds, fd_set * exceptfds,
@@ -213,4 +213,17 @@
 	}
 
 	return (ret);
+}
+
+int 
+__select(int numfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
+	struct timeval *timeout)
+{
+	int ret;
+
+	_thread_enter_cancellation_point();
+	ret = _select(numfds, readfds, writefds, exceptfds, timeout);
+	_thread_leave_cancellation_point();
+
+	return ret;
 }
Index: uthread_wait4.c
===================================================================
RCS file: /home/ncvs/src/lib/libc_r/uthread/uthread_wait4.c,v
retrieving revision 1.13
diff -u -r1.13 uthread_wait4.c
--- uthread_wait4.c	10 Apr 2001 04:19:20 -0000	1.13
+++ uthread_wait4.c	2 May 2002 18:58:14 -0000
@@ -38,7 +38,7 @@
 #include <pthread.h>
 #include "pthread_private.h"
 
-__weak_reference(_wait4, wait4);
+__weak_reference(__wait4, wait4);
 
 pid_t
 _wait4(pid_t pid, int *istat, int options, struct rusage * rusage)
@@ -67,4 +67,16 @@
 	_thread_kern_sig_undefer();
 
 	return (ret);
+}
+
+pid_t
+__wait4(pid_t pid, int *istat, int options, struct rusage *rusage)
+{
+	pid_t ret;
+
+	_thread_enter_cancellation_point();
+	ret = _wait4(pid, istat, options, rusage);
+	_thread_leave_cancellation_point();
+
+	return ret;
 }
Index: uthread_writev.c
===================================================================
RCS file: /home/ncvs/src/lib/libc_r/uthread/uthread_writev.c,v
retrieving revision 1.18
diff -u -r1.18 uthread_writev.c
--- uthread_writev.c	10 Apr 2001 04:19:20 -0000	1.18
+++ uthread_writev.c	2 May 2002 18:58:14 -0000
@@ -42,7 +42,7 @@
 #include <pthread.h>
 #include "pthread_private.h"
 
-__weak_reference(_writev, writev);
+__weak_reference(__writev, writev);
 
 ssize_t
 _writev(int fd, const struct iovec * iov, int iovcnt)
@@ -201,4 +201,16 @@
 		free(p_iov);
 
 	return (ret);
+}
+
+ssize_t
+__writev(int fd, const struct iovec *iov, int iovcnt)
+{
+	ssize_t ret;
+
+	_thread_enter_cancellation_point();
+	ret = _writev(fd, iov, iovcnt);
+	_thread_leave_cancellation_point();
+
+	return ret;
 }

Comment 3 Archie Cobbs freebsd_committer

2002-05-05 05:37:02 UTC

Responsible Changed
From-To: freebsd-bugs->archie

Assign bug to me since I'm fixing it.

Comment 4 Archie Cobbs freebsd_committer

2002-05-07 19:49:14 UTC

State Changed
From-To: open->closed

Fixed in -current and -stable.