Re: close(2) while accept(2) is blocked
Bakul Shah wrote this message on Sat, Mar 30, 2013 at 13:22 -0700: > On Sat, 30 Mar 2013 09:14:34 PDT John-Mark Gurney wrote: > > > > As someone else pointed out in this thread, if a userland program > > depends upon this behavior, it has a race condition in it... > > > > Thread 1Thread 2Thread 3 > > enters routine to read > > enters routine to close > > calls close(3) > > open() returns 3 > > does read(3) for orignal fd > > > > How can the original threaded program ensure that thread 2 doesn't > > create a new fd in between? So even if you use a lock, this won't > > help, because as far as I know, there is no enter read and unlock > > mutex call yet... > > It is worse. Consider: > > fd = open(file,...); > read(fd, ...); > > No guarantee read() gets data from the same opened file! > Another thread could've come along, closed fd and pointed it > to another file. So nothing is safe. Might as well stop using > threads, right?! Nope, you need your threads to cooperate w/ either locks or some ownership mechanism like token passing... As long as only one thread own the fd, you won't have troubles... > We are talking about cooperative threads where you don't have > to assume the worst case. Here not being notified on a close Multiple people have said when threads cooperate without locks, but no one has given a good example where they do... The cases you list below are IMO special (and extreme) cases... We were talking about "cooperating" threads in a general purpose langage like Java where you can not depend upon your other threads doing limited amount of work... > event can complicate things. As an example, I have done > something like this in the past: A frontend process validating > TCP connections and then passing on valid TCP connections to > another process for actual service (via sendmsg() over a unix > domain). All the worker threads in service process can do a > recvmsg() on the same fd. They process whatever tcp connection > they get. Now what happens when the frontend process is > restarted for some reason? All the worker threads need to > eventually reconnect to a new unix domain posted by the new > frontend process. You can handle this multiple ways but > terminating all the blocking syscalls on the now invalid fd is > the simplest solution from a user perspective. This'd make an interesting race... Not sure if it could happen, but close(), closes the receiving fd, but another thread is in the process of receiving an fd and puts the new fd in the close of the now closed unix domain socket... I can draw a more detailed diagram if you want.. > > I decided long ago that this is only solvable by proper use of locking > > and ensuring that if you call close (the syscall), that you do not have > > any other thread that may use the fd. It's the close routine's (not > > syscall) function to make sure it locks out other threads and all other > > are out of the code path that will use the fd before it calls close.. > > If you lock before close(), you have to lock before every > other syscall on that fd. That complicates userland coding and > slows down things when this can be handled more simply in the > kernel. There is "ownership" that can be passed, such as via kqueue _ONESHOT w/ multiple threads which allows you to avoid locking and only one thread ever owns the fd... > Another usecase is where N worker threads all accept() on the > same fd. Single threading using a lock defeats any performance > gain. In this case you still need to do something special since what happens if one of the worker threads opens a file and/or listen/accept socket as part of it's work? Thread 1Thread 2Thread 3 about to call accept but after flag check sets flag that close is going to be called accept connect process connection close listen'd socket open socket as part of processing gets same fd calls listen on socket calls accept on socket And there we have it, a race condition... You can't always guarantee what your worker threads do, if you can, it's good, but we need to make sure that beginner programs don't get into traps like these... They can easily make the mistake of saying, well, since close kicks all my threads out, I'll just do that instead of making sure that they don't have a race condition... > > If someone could describe how this new eject a person from read could > > be done in a race safe way, then I'd say go ahead w/ it... Otherwise > > we're just moving the race around, and letting people think that they > > have solved the problem when they haven't... > > In general it just makes sens
Re: close(2) while accept(2) is blocked
On Thu, Apr 04, 2013 at 08:43:17PM +0300, Andriy Gapon wrote:
> on 01/04/2013 18:22 John Baldwin said the following:
> > I think you need to split the 'struct file' reference count into two
> > different counts similar to the how we have vref/vrele vs
> > vhold/vdrop for vnodes. The fget for accept and probably most other
> > system calls should probably be equivalent to vhold, whereas things
> > like open/dup (and storing an fd in a cmsg) should be more like
> > vref. close() should then be a vrele().
> This model makes perfect sense.
> Unfortunately, ENOTIME to work on this.
This looks like it can work but I don't know whether it's worth the
trouble.
> Meanwhile I am using the following patch specific to local domain
> sockets, accept(2) and shutdown(2). Turns out that the problematic
> application does both shutdown(RDWR) and close(2), but that doesn't
> help on FreeBSD.
> BTW, this is the application:
> http://thread.gmane.org/gmane.os.freebsd.devel.office/1754
> The patch does help.
> Author: Andriy Gapon
> Date: Thu Mar 28 20:08:13 2013 +0200
>
> [test!] uipc_shutdown: use soisdisconnected instead of socantsendmore
>
> So that in addition to disabling sends we also wake up threads blocked
> in accept (on so_timeo in general).
>
> diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c
> index 9b60eab..e93d46c 100644
> --- a/sys/kern/uipc_usrreq.c
> +++ b/sys/kern/uipc_usrreq.c
> @@ -1074,7 +1074,7 @@ uipc_shutdown(struct socket *so)
>
> UNP_LINK_WLOCK();
> UNP_PCB_LOCK(unp);
> - socantsendmore(so);
> + soisdisconnected(so);
> unp_shutdown(unp);
> UNP_PCB_UNLOCK(unp);
> UNP_LINK_WUNLOCK();
I think this patch makes shutdown(SHUT_WR) on unix domain sockets act
like shutdown(SHUT_RDWR), i.e. receives are incorrectly denied.
The below patch also makes shutdown(SHUT_RDWR) abort a blocking accept
on a unix domain socket, but it should work for all domains:
Index: sys/kern/uipc_socket.c
===
--- sys/kern/uipc_socket.c (revision 248873)
+++ sys/kern/uipc_socket.c (working copy)
@@ -2428,9 +2428,11 @@ soshutdown(struct socket *so, int how)
sorflush(so);
if (how != SHUT_RD) {
error = (*pr->pr_usrreqs->pru_shutdown)(so);
+ wakeup(&so->so_timeo);
CURVNET_RESTORE();
return (error);
}
+ wakeup(&so->so_timeo);
CURVNET_RESTORE();
return (0);
}
A blocking accept (and some other operations) is waiting on
&so->so_timeo. Once it wakes up, it will detect the SBS_CANTRCVMORE bit.
A spurious wakeup on so->so_timeo appears harmless (sleep retried)
except when lingering on close (SO_LINGER) so this should be fairly
safe.
--
Jilles Tjoelker
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
Re: close(2) while accept(2) is blocked
on 01/04/2013 18:22 John Baldwin said the following: > I think you need to split the 'struct file' reference count into two different > counts similar to the how we have vref/vrele vs vhold/vdrop for vnodes. The > fget for accept and probably most other system calls should probably be > equivalent > to vhold, whereas things like open/dup (and storing an fd in a cmsg) should be > more like vref. close() should then be a vrele(). This model makes perfect sense. Unfortunately, ENOTIME to work on this. Meanwhile I am using the following patch specific to local domain sockets, accept(2) and shutdown(2). Turns out that the problematic application does both shutdown(RDWR) and close(2), but that doesn't help on FreeBSD. BTW, this is the application: http://thread.gmane.org/gmane.os.freebsd.devel.office/1754 The patch does help. Author: Andriy Gapon Date: Thu Mar 28 20:08:13 2013 +0200 [test!] uipc_shutdown: use soisdisconnected instead of socantsendmore So that in addition to disabling sends we also wake up threads blocked in accept (on so_timeo in general). diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c index 9b60eab..e93d46c 100644 --- a/sys/kern/uipc_usrreq.c +++ b/sys/kern/uipc_usrreq.c @@ -1074,7 +1074,7 @@ uipc_shutdown(struct socket *so) UNP_LINK_WLOCK(); UNP_PCB_LOCK(unp); - socantsendmore(so); + soisdisconnected(so); unp_shutdown(unp); UNP_PCB_UNLOCK(unp); UNP_LINK_WUNLOCK(); -- Andriy Gapon ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
Re: close(2) while accept(2) is blocked
on 30/03/2013 18:14 John-Mark Gurney said the following: > As someone else pointed out in this thread, if a userland program > depends upon this behavior, it has a race condition in it... > > Thread 1 Thread 2Thread 3 > enters routine to read > enters routine to close > calls close(3) > open() returns 3 > does read(3) for orignal fd > > How can the original threaded program ensure that thread 2 doesn't > create a new fd in between? So even if you use a lock, this won't > help, because as far as I know, there is no enter read and unlock > mutex call yet... > > I decided long ago that this is only solvable by proper use of locking > and ensuring that if you call close (the syscall), that you do not have > any other thread that may use the fd. It's the close routine's (not > syscall) function to make sure it locks out other threads and all other > are out of the code path that will use the fd before it calls close.. > > If someone could describe how this new eject a person from read could > be done in a race safe way, then I'd say go ahead w/ it... Otherwise > we're just moving the race around, and letting people think that they > have solved the problem when they haven't... > > I think I remeber another thread about this from a year or two ago, > but I couldn't find it... If someone finds it, posting a link would > be nice.. > I wish to abstract as much as possible from how an application may use, misuse or even abuse the close+ interaction. But I think that the behavior that provides more information / capabilities is preferable over the behavior that does not. E.g. your example above does not apply to a utility that has only two threads. The "three threads" problem can also be solved if all the threads cooperate. But as I've said. -- Andriy Gapon ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
Re: close(2) while accept(2) is blocked
On Thursday, March 28, 2013 12:54:31 pm Andriy Gapon wrote: > > So, this started as a simple question, but the answer was quite unexpected to > me. > > Let's say we have an opened and listen-ed socket and let's assume that we know > that one thread is blocked in accept(2) and another thread is calling > close(2). > What is going to happen? > > Turns out that practically nothing. For kernel the close call would be > almost a nop. > My understanding is this: > - when socket is created, its reference count is 1 > - when accept(2) is called, fget in kernel increments the reference count > (kept in > an associated struct file) > - when close(2) is called, the reference count is decremented > > The reference count is still greater than zero, so fdrop does not call > fo_close. > That means that in the case of a socket soclose is not called. > > I am sure that the reference counting in this case is absolutely correct with > respect to managing kernel side structures. But I am not that it is correct > with > respect to hiding the explicit close(2) call from other threads that may be > waiting on the socket. > In other words, I am not sure if fo_close is supposed to signify that there > are no > uses of a file, or that userland close-d the file. Or perhaps these should > be two > different methods. > > Additional note is that shutdown(2) doesn't wake up the thread in accept(2) > either. At least that's true for unix domain sockets. > Not sure if this is a bug. > > But the summary seems to be is that currently it is not possible to break a > thread > out of accept(2) (at least without resorting to signals). I think you need to split the 'struct file' reference count into two different counts similar to the how we have vref/vrele vs vhold/vdrop for vnodes. The fget for accept and probably most other system calls should probably be equivalent to vhold, whereas things like open/dup (and storing an fd in a cmsg) should be more like vref. close() should then be a vrele(). -- John Baldwin ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
Re: close(2) while accept(2) is blocked
On Sat, 30 Mar 2013 09:14:34 PDT John-Mark Gurney wrote: > > As someone else pointed out in this thread, if a userland program > depends upon this behavior, it has a race condition in it... > > Thread 1 Thread 2Thread 3 > enters routine to read > enters routine to close > calls close(3) > open() returns 3 > does read(3) for orignal fd > > How can the original threaded program ensure that thread 2 doesn't > create a new fd in between? So even if you use a lock, this won't > help, because as far as I know, there is no enter read and unlock > mutex call yet... It is worse. Consider: fd = open(file,...); read(fd, ...); No guarantee read() gets data from the same opened file! Another thread could've come along, closed fd and pointed it to another file. So nothing is safe. Might as well stop using threads, right?! We are talking about cooperative threads where you don't have to assume the worst case. Here not being notified on a close event can complicate things. As an example, I have done something like this in the past: A frontend process validating TCP connections and then passing on valid TCP connections to another process for actual service (via sendmsg() over a unix domain). All the worker threads in service process can do a recvmsg() on the same fd. They process whatever tcp connection they get. Now what happens when the frontend process is restarted for some reason? All the worker threads need to eventually reconnect to a new unix domain posted by the new frontend process. You can handle this multiple ways but terminating all the blocking syscalls on the now invalid fd is the simplest solution from a user perspective. > I decided long ago that this is only solvable by proper use of locking > and ensuring that if you call close (the syscall), that you do not have > any other thread that may use the fd. It's the close routine's (not > syscall) function to make sure it locks out other threads and all other > are out of the code path that will use the fd before it calls close.. If you lock before close(), you have to lock before every other syscall on that fd. That complicates userland coding and slows down things when this can be handled more simply in the kernel. Another usecase is where N worker threads all accept() on the same fd. Single threading using a lock defeats any performance gain. > If someone could describe how this new eject a person from read could > be done in a race safe way, then I'd say go ahead w/ it... Otherwise > we're just moving the race around, and letting people think that they > have solved the problem when they haven't... In general it just makes sense to notify everyone waiting on something that the situation has changed and they are going to have to wait forever. The kernel should already have the necessary information about which threads are sleeping on a fd. Wake them all up. On being awakened they see that the fd is no longer valid and all return with a count of data already read or -1 and EBADF. Doing the equivalent in userland is complicated. Carl has pointed out how BSD and Linux have required a workaround compared to Solaris and OS X (in Java and IIRC, the Go runtime). Seems like we have a number of usecases and this is something worth fixing. ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
Re: close(2) while accept(2) is blocked
> As someone else pointed out in this thread, if a userland program > depends upon this behavior, it has a race condition in it... > > Thread 1 Thread 2Thread 3 > enters routine to read > enters routine to close > calls close(3) > open() returns 3 > does read(3) for orignal fd > > How can the original threaded program ensure that thread 2 doesn't > create a new fd in between? So even if you use a lock, this won't > help, because as far as I know, there is no enter read and unlock > mutex call yet... > > I decided long ago that this is only solvable by proper use of locking > and ensuring that if you call close (the syscall), that you do not have > any other thread that may use the fd. It's the close routine's (not > syscall) function to make sure it locks out other threads and all other > are out of the code path that will use the fd before it calls close.. > > If someone could describe how this new eject a person from read could > be done in a race safe way, then I'd say go ahead w/ it... Otherwise > we're just moving the race around, and letting people think that they > have solved the problem when they haven't... Right. The only "safe" way is to have all blocking syscalls on the same fd in the same process return to userland. This would need to be initiated in the close() syscall. Btw. Threads aren't the only scenario. A signal handler can also close the fd. Maybe not advised, but I have used this "technique" to force a return from a blocking accept() call since about FBSD4.x Mark. ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
Re: close(2) while accept(2) is blocked
Bakul Shah wrote this message on Fri, Mar 29, 2013 at 16:54 -0700: > On Fri, 29 Mar 2013 14:30:59 PDT Carl Shapiro wrote: > > > > In other operating systems, such as Solaris and MacOS X, closing the > > descriptor causes blocked system calls to return with an error. > > What happens if you select() on a socket and another thread > closes this socket? Ideally select() should return (with > EINTR?) so that the blocking thread can some cleanup action. > And if you do that, the blocking accept() case is not really > different. > > There is no point in *not* telling blocking threads that the > descriptor they're waiting on is one EBADF and nothing is > going to happen. > > > It is not obvious whether there is any benefit to having the current > > blocking behaviour. > > This may need some new kernel code but IMHO this is worth fixing. As someone else pointed out in this thread, if a userland program depends upon this behavior, it has a race condition in it... Thread 1Thread 2Thread 3 enters routine to read enters routine to close calls close(3) open() returns 3 does read(3) for orignal fd How can the original threaded program ensure that thread 2 doesn't create a new fd in between? So even if you use a lock, this won't help, because as far as I know, there is no enter read and unlock mutex call yet... I decided long ago that this is only solvable by proper use of locking and ensuring that if you call close (the syscall), that you do not have any other thread that may use the fd. It's the close routine's (not syscall) function to make sure it locks out other threads and all other are out of the code path that will use the fd before it calls close.. If someone could describe how this new eject a person from read could be done in a race safe way, then I'd say go ahead w/ it... Otherwise we're just moving the race around, and letting people think that they have solved the problem when they haven't... I think I remeber another thread about this from a year or two ago, but I couldn't find it... If someone finds it, posting a link would be nice.. -- John-Mark Gurney Voice: +1 415 225 5579 "All that I will do, has been done, All that I have, has not." ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
Re: close(2) while accept(2) is blocked
On Fri, 29 Mar 2013 14:30:59 PDT Carl Shapiro wrote: > > In other operating systems, such as Solaris and MacOS X, closing the > descriptor causes blocked system calls to return with an error. What happens if you select() on a socket and another thread closes this socket? Ideally select() should return (with EINTR?) so that the blocking thread can some cleanup action. And if you do that, the blocking accept() case is not really different. There is no point in *not* telling blocking threads that the descriptor they're waiting on is one EBADF and nothing is going to happen. > It is not obvious whether there is any benefit to having the current > blocking behaviour. This may need some new kernel code but IMHO this is worth fixing. ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
Re: close(2) while accept(2) is blocked
On Thu, Mar 28, 2013 at 9:54 AM, Andriy Gapon wrote: > But the summary seems to be is that currently it is not possible to break > a thread > out of accept(2) (at least without resorting to signals). > This is a known problem for Java. Closing a socket that another thread is block on is supposed to unblock a thread and throw a SocketException. http://docs.oracle.com/javase/7/docs/api/java/net/Socket.html#close() In other operating systems, such as Solaris and MacOS X, closing the descriptor causes blocked system calls to return with an error. FreeBSD (and Linux) do not behave this way. There, a JVM must do extra bookkeeping to associate file descriptors with threads blocked on the descriptor. This allows the close method to identify blocked threads and send them a signal. On those threads the caller of the blocking method must further distinguish an error return as being caused by a signal sent on behalf of close. It is not obvious whether there is any benefit to having the current blocking behaviour. Threads are an afterthought in UNIX so this could just be an oversight. It would make a high-level language runtimes simpler if FreeBSD behaved more like Solaris and MacOS X. ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
Re: close(2) while accept(2) is blocked
On Thu, Mar 28, 2013 at 06:54:31PM +0200, Andriy Gapon wrote: > So, this started as a simple question, but the answer was quite > unexpected to me. > Let's say we have an opened and listen-ed socket and let's assume that > we know that one thread is blocked in accept(2) and another thread is > calling close(2). What is going to happen? > Turns out that practically nothing. For kernel the close call would > be almost a nop. > My understanding is this: > - when socket is created, its reference count is 1 > - when accept(2) is called, fget in kernel increments the reference > count (kept in an associated struct file) > - when close(2) is called, the reference count is decremented > The reference count is still greater than zero, so fdrop does not call > fo_close. That means that in the case of a socket soclose is not > called. > I am sure that the reference counting in this case is absolutely > correct with respect to managing kernel side structures. I agree this is expected and correct from the kernel point of view. > But I am not that it is correct with respect to hiding the explicit > close(2) call from other threads that may be waiting on the socket. In > other words, I am not sure if fo_close is supposed to signify that > there are no uses of a file, or that userland close-d the file. Or > perhaps these should be two different methods. It would be possible to keep track of the file descriptor number but I think it is not worth the large amount of extra code. Keeping track of file descriptor number would be necessary to interrupt waits after a close or dup2 on the file descriptor that was passed to the blocking call, even if the object remains open on a different file descriptor number or in a different process. Also, most people would use the new functionality incorrectly anyway. A close() on a file descriptor another thread is using is risky since it is in most cases impossible to prove that the other thread is in fact blocked on the file descriptor and not preempted right before making the system call. In the latter case, the other thread might accept a connection from a different socket created later. A dup2() of /dev/null onto the file descriptor would be safer. > Additional note is that shutdown(2) doesn't wake up the thread in > accept(2) either. At least that's true for unix domain sockets. > Not sure if this is a bug. I think it is a bug. It works properly for IPv4 TCP sockets. The resulting error is [ECONNABORTED] for a blocking socket which likely leads to infinite looping if the thread does not know about the shutdown(2) (because that error normally means the accept should be retried later). For a non-blocking socket the error is [EWOULDBLOCK] which also leads to infinite looping and is certainly wrong because select/poll do report the socket as readable. Both of these are in kern_accept() in sys/kern/uipc_syscalls.c. POSIX does not say which error code we should return here but these two are almost certainly wrong (it is usable for waking up threads stuck in accept() if those threads check a variable after every accept() failure and do not rely on the exact value of errno). Linux returns [EINVAL] for both blocking and non-blocking sockets, probably from the POSIX error condition "The socket is not accepting connections." In our man page that error condition is formulated "listen(2) has not been called on the socket descriptor." which is clearly not the case. Also, I think a non-blocking accept() should immediately fail with the head->so_error if it is set, rather than returning [EWOULDBLOCK] until another connection arrives. Likewise, filt_solisten() in sys/kern/uipc_socket.c only returns true if there is a connection, not if there was an error or shutdown() has been called. On the other hand, sopoll_generic() looks correct. Error reporting on non-blocking accept() might usefully be postponed until there is a connection or the socket has been shut down, to reduce context switches. > But the summary seems to be is that currently it is not possible to > break a thread out of accept(2) (at least without resorting to > signals). Pthread cancellation works better than raw signals for this use case. In a proper implementation such as in FreeBSD 9.0 or newer and used properly, it allows avoiding the resource leak that may happen when calling longjmp() or pthread_exit() in a signal handler just after accept() has created a new socket. -- Jilles Tjoelker ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
