NetBSD Problem Report #26567

Received: (qmail 25185 invoked by uid 605); 6 Aug 2004 06:43:00 -0000
Message-Id: <1091774573.443024.5265.nullmailer@yamt.dyndns.org>
Date: Fri, 06 Aug 2004 15:42:53 +0900
From: yamt@mwd.biglobe.ne.jp
Sender: gnats-bugs-owner@NetBSD.org
Reply-To: yamt@mwd.biglobe.ne.jp
To: gnats-bugs@gnats.NetBSD.org
Subject: closef deadlock with threads
X-Send-Pr-Version: 3.95

>Number:         26567
>Category:       kern
>Synopsis:       closef deadlock with threads
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Aug 06 06:44:00 +0000 2004
>Closed-Date:    Sun Dec 20 10:43:12 +0000 2009
>Last-Modified:  Sun Dec 20 10:43:12 +0000 2009
>Originator:     YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
>Release:        NetBSD 2.0G
>Organization:

>Environment:


System: NetBSD kaeru 2.0G NetBSD 2.0G (build.kaeru) #1594: Thu Aug 5 06:34:13 JST 2004 takashi@kaeru:/home/takashi/work/kernel/build.kaeru i386
Architecture: i386
Machine: i386
>Description:
	our descriptor handling code has an assumption that
	a file is kept in a descriptor table during the most operations.
	however, it no longer be held these days due to pthread and clone(2).

>How-To-Repeat:
	run the following test code and see that
	the main thread blocks at the first close(2) while
	the test() thread blocks in read(2) forever.

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

int fds[2];

void *
test(void *dummy)
{
	char buf[1];
	int rv;

	rv = read(fds[0], buf, 1);		/* WILL BLOCK HERE */
	if (rv == 0) {
		fprintf(stderr, "EOF\n");
	} else if (rv == -1) {
		perror("read");
		exit(1);
	} else {
		fprintf(stderr, "%d\n", rv);
		exit(1);
	}

	return NULL;
}

int
main()
{
	pthread_t t;

	if (pipe(fds)) {
		perror("pipe");
		exit(1);
	}
	if (pthread_create(&t, NULL, test, NULL)) {
		perror("create");
		exit(1);
	}
	fprintf(stderr, "sleep\n");
	sleep(1);
	fprintf(stderr, "close\n");
	if (close(fds[0])) {			/* WILL BLOCK HERE */
		perror("close");
		exit(1);
	}
	fprintf(stderr, "close writer\n");
	if (close(fds[1])) {
		perror("close");
		exit(1);
	}
	fprintf(stderr, "join\n");
	if (pthread_join(t, NULL)) {
		perror("join");
		exit(1);
	}
	exit(0);
}

>Fix:
	unify f_count and f_usecount?

>Release-Note:
>Audit-Trail:

From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
To: gnats-bugs@gnats.NetBSD.org
Cc:  
Subject: Re: kern/26567: closef deadlock with threads
Date: Wed, 11 Aug 2004 17:18:56 +0900

 hm, i misunderstood the code history.
 i thought that the closef code was somewhat ancient.
 actually, it has been changed to the current form to deal with threads.

 anyway, it's better not to deadlock in this test case.
 FWIW, the read(2) returns EBADF on SunOS 5.8
 and OTOH close doesn't block on Linux.

 YAMAMOTO Takashi
Responsible-Changed-From-To: kern-bug-people->ad
Responsible-Changed-By: ad@NetBSD.org
Responsible-Changed-When: Fri, 21 Mar 2008 16:25:00 +0000
Responsible-Changed-Why:
take


Responsible-Changed-From-To: ad->kern-bug-people
Responsible-Changed-By: ad@NetBSD.org
Responsible-Changed-When: Tue, 29 Apr 2008 13:37:58 +0000
Responsible-Changed-Why:
Don't have time to look at this right now.


From: David Laight <david@l8s.co.uk>
To: gnats-bugs@gnats.netbsd.org
Cc: 
Subject: RE: PR/26567
Date: Sun, 6 Dec 2009 19:22:59 +0000

 The 'fd' part of this was implemented by ad@ in Apr '09.
 However the fo_drain() function was only implemented for
 sockets, not pipes, fifos, etc.

 	David

 -- 
 David Laight: david@l8s.co.uk

From: David Laight <dsl@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/26567 CVS commit: src/sys
Date: Sat, 12 Dec 2009 21:28:04 +0000

 Module Name:	src
 Committed By:	dsl
 Date:		Sat Dec 12 21:28:04 UTC 2009

 Modified Files:
 	src/sys/kern: sys_pipe.c
 	src/sys/sys: pipe.h

 Log Message:
 Add support for unblocking read/write when close called.
 Fixes PR/26567 for pipes.
 (NB ad backed out the fix for sockets)


 To generate a diff of this commit:
 cvs rdiff -u -r1.122 -r1.123 src/sys/kern/sys_pipe.c
 cvs rdiff -u -r1.29 -r1.30 src/sys/sys/pipe.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: David Laight <dsl@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/26567 CVS commit: src/sys
Date: Sun, 13 Dec 2009 18:27:02 +0000

 Module Name:	src
 Committed By:	dsl
 Date:		Sun Dec 13 18:27:02 UTC 2009

 Modified Files:
 	src/sys/kern: sys_pipe.c
 	src/sys/sys: pipe.h

 Log Message:
 Revert most of the previous change.
 Only one fd needs clobbering, not all fds that reference the pipe.
 This may be what ad@ realised when he tried to add the same code to
 sockets. Unfixes part of PR/26567.


 To generate a diff of this commit:
 cvs rdiff -u -r1.123 -r1.124 src/sys/kern/sys_pipe.c
 cvs rdiff -u -r1.30 -r1.31 src/sys/sys/pipe.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: David Laight <dsl@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/26567 CVS commit: src/sys/kern
Date: Sun, 13 Dec 2009 20:02:24 +0000

 Module Name:	src
 Committed By:	dsl
 Date:		Sun Dec 13 20:02:23 UTC 2009

 Modified Files:
 	src/sys/kern: sys_pipe.c

 Log Message:
 Another, better, fix for PR/26567.
 Only sleep once within each pipe_read/pipe_write call.
 If there is no data/space available after we wakeup return ERESTART so
 then the 'fd' number is validated again.
 A simple broadcast of the cvs is then enough to evict the correct threads
 when close() is called from an active thread.


 To generate a diff of this commit:
 cvs rdiff -u -r1.124 -r1.125 src/sys/kern/sys_pipe.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: David Laight <dsl@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/26567 CVS commit: src/sys
Date: Sun, 20 Dec 2009 09:36:07 +0000

 Module Name:	src
 Committed By:	dsl
 Date:		Sun Dec 20 09:36:06 UTC 2009

 Modified Files:
 	src/sys/arch/xen/xen: xenevt.c
 	src/sys/compat/svr4: svr4_net.c
 	src/sys/compat/svr4_32: svr4_32_net.c
 	src/sys/dev/dmover: dmover_io.c
 	src/sys/dev/putter: putter.c
 	src/sys/kern: kern_descrip.c kern_drvctl.c kern_event.c sys_mqueue.c
 	    sys_pipe.c sys_socket.c uipc_socket.c uipc_syscalls.c vfs_vnops.c
 	src/sys/net: bpf.c if_tap.c
 	src/sys/opencrypto: cryptodev.c
 	src/sys/sys: file.h pipe.h socketvar.h

 Log Message:
 If a multithreaded app closes an fd while another thread is blocked in
 read/write/accept, then the expectation is that the blocked thread will
 exit and the close complete.
 Since only one fd is affected, but many fd can refer to the same file,
 the close code can only request the fs code unblock with ERESTART.
 Fixed for pipes and sockets, ERESTART will only be generated after such
 a close - so there should be no change for other programs.
 Also rename fo_abort() to fo_restart() (this used to be fo_drain()).
 Fixes PR/26567


 To generate a diff of this commit:
 cvs rdiff -u -r1.35 -r1.36 src/sys/arch/xen/xen/xenevt.c
 cvs rdiff -u -r1.57 -r1.58 src/sys/compat/svr4/svr4_net.c
 cvs rdiff -u -r1.20 -r1.21 src/sys/compat/svr4_32/svr4_32_net.c
 cvs rdiff -u -r1.36 -r1.37 src/sys/dev/dmover/dmover_io.c
 cvs rdiff -u -r1.25 -r1.26 src/sys/dev/putter/putter.c
 cvs rdiff -u -r1.201 -r1.202 src/sys/kern/kern_descrip.c
 cvs rdiff -u -r1.30 -r1.31 src/sys/kern/kern_drvctl.c
 cvs rdiff -u -r1.67 -r1.68 src/sys/kern/kern_event.c
 cvs rdiff -u -r1.28 -r1.29 src/sys/kern/sys_mqueue.c
 cvs rdiff -u -r1.126 -r1.127 src/sys/kern/sys_pipe.c
 cvs rdiff -u -r1.62 -r1.63 src/sys/kern/sys_socket.c
 cvs rdiff -u -r1.195 -r1.196 src/sys/kern/uipc_socket.c
 cvs rdiff -u -r1.137 -r1.138 src/sys/kern/uipc_syscalls.c
 cvs rdiff -u -r1.167 -r1.168 src/sys/kern/vfs_vnops.c
 cvs rdiff -u -r1.149 -r1.150 src/sys/net/bpf.c
 cvs rdiff -u -r1.61 -r1.62 src/sys/net/if_tap.c
 cvs rdiff -u -r1.50 -r1.51 src/sys/opencrypto/cryptodev.c
 cvs rdiff -u -r1.69 -r1.70 src/sys/sys/file.h
 cvs rdiff -u -r1.31 -r1.32 src/sys/sys/pipe.h
 cvs rdiff -u -r1.122 -r1.123 src/sys/sys/socketvar.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: dsl@NetBSD.org
State-Changed-When: Sun, 20 Dec 2009 10:43:12 +0000
State-Changed-Why:
Fixed on head for pipes and sockets.
Too many other changes are required to consider a pullup for netbsd 5


>Unformatted:

Home	PR Database Search