]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: fix use after close race in Solaris network poller
authorAram Hăvărneanu <aram@mgk.ro>
Fri, 14 Mar 2014 13:53:05 +0000 (17:53 +0400)
committerDmitriy Vyukov <dvyukov@google.com>
Fri, 14 Mar 2014 13:53:05 +0000 (17:53 +0400)
The Solaris network poller uses event ports, which are
level-triggered. As such, it has to re-arm itself after each
wakeup. The arming mechanism (which runs in its own thread) raced
with the closing of a file descriptor happening in a different
thread. When a network file descriptor is about to be closed,
the network poller is awaken to give it a chance to remove its
association with the file descriptor. Because the poller always
re-armed itself, it raced with code that closed the descriptor.

This change makes the network poller check before re-arming if
the file descriptor is about to be closed, in which case it will
ignore the re-arming request. It uses the per-PollDesc lock in
order to serialize access to the PollDesc.

This change also adds extensive documentation describing the
Solaris implementation of the network poller.

Fixes #7410.

LGTM=dvyukov, iant
R=golang-codereviews, bradfitz, iant, dvyukov, aram.h, gobot
CC=golang-codereviews
https://golang.org/cl/69190044

src/pkg/runtime/netpoll.goc
src/pkg/runtime/netpoll_solaris.c
src/pkg/runtime/runtime.h

index 77ddde9d60160e5c5d9d2977be5888ff2a0f087f..7b3d16d02da4c27a164d7142b92765f27a24226d 100644 (file)
@@ -263,6 +263,24 @@ runtime·netpolluser(PollDesc *pd)
        return &pd->user;
 }
 
+bool
+runtime·netpollclosing(PollDesc *pd)
+{
+       return pd->closing;
+}
+
+void
+runtime·netpolllock(PollDesc *pd)
+{
+       runtime·lock(pd);
+}
+
+void
+runtime·netpollunlock(PollDesc *pd)
+{
+       runtime·unlock(pd);
+}
+
 // make pd ready, newly runnable goroutines (if any) are enqueued info gpp list
 void
 runtime·netpollready(G **gpp, PollDesc *pd, int32 mode)
index f745f234356b5df2f5da9564d51662c4e03589a3..a2631a8ab9ae4d3eb2edf4a4abdea08c5de10cc8 100644 (file)
@@ -7,6 +7,67 @@
 #include "defs_GOOS_GOARCH.h"
 #include "os_GOOS.h"
 
+// Solaris runtime-integrated network poller.
+// 
+// Solaris uses event ports for scalable network I/O. Event
+// ports are level-triggered, unlike epoll and kqueue which
+// can be configured in both level-triggered and edge-triggered
+// mode. Level triggering means we have to keep track of a few things
+// ourselves. After we receive an event for a file descriptor,
+// it's our responsibility to ask again to be notified for future
+// events for that descriptor. When doing this we must keep track of
+// what kind of events the goroutines are currently interested in,
+// for example a fd may be open both for reading and writing.
+// 
+// A description of the high level operation of this code
+// follows. Networking code will get a file descriptor by some means
+// and will register it with the netpolling mechanism by a code path
+// that eventually calls runtime·netpollopen. runtime·netpollopen
+// calls port_associate with an empty event set. That means that we
+// will not receive any events at this point. The association needs
+// to be done at this early point because we need to process the I/O
+// readiness notification at some point in the future. If I/O becomes
+// ready when nobody is listening, when we finally care about it,
+// nobody will tell us anymore.
+// 
+// Beside calling runtime·netpollopen, the networking code paths
+// will call runtime·netpollarm each time goroutines are interested
+// in doing network I/O. Because now we know what kind of I/O we
+// are interested in (reading/writting), we can call port_associate
+// passing the correct type of event set (POLLIN/POLLOUT). As we made
+// sure to have already associated the file descriptor with the port,
+// when we now call port_associate, we will unblock the main poller
+// loop (in runtime·netpoll) right away if the socket is actually
+// ready for I/O.
+// 
+// The main poller loop runs in its own thread waiting for events
+// using port_getn. When an event happens, it will tell the scheduler
+// about it using runtime·netpollready. Besides doing this, it must
+// also re-associate the events that were not part of this current
+// notification with the file descriptor. Failing to do this would
+// mean each notification will prevent concurrent code using the
+// same file descriptor in parallel.
+// 
+// The logic dealing with re-associations is encapsulated in
+// runtime·netpollupdate. This function takes care to associate the
+// descriptor only with the subset of events that were previously
+// part of the association, except the one that just happened. We
+// can't re-associate with that right away, because event ports
+// are level triggered so it would cause a busy loop. Instead, that
+// association is effected only by the runtime·netpollarm code path,
+// when Go code actually asks for I/O.
+// 
+// The open and arming mechanisms are serialized using the lock
+// inside PollDesc. This is required because the netpoll loop runs
+// asynchonously in respect to other Go code and by the time we get
+// to call port_associate to update the association in the loop, the
+// file descriptor might have been closed and reopened already. The
+// lock allows runtime·netpollupdate to be called synchronously from
+// the loop thread while preventing other threads operating to the
+// same PollDesc, so once we unblock in the main loop, until we loop
+// again we know for sure we are always talking about the same file
+// descriptor and can safely access the data we want (the event set).
+
 #pragma dynimport libc·fcntl fcntl "libc.so"
 #pragma dynimport libc·port_create port_create "libc.so"
 #pragma dynimport libc·port_associate port_associate "libc.so"
@@ -71,10 +132,19 @@ runtime·netpollinit(void)
 int32
 runtime·netpollopen(uintptr fd, PollDesc *pd)
 {
-       uint32 events = POLLIN | POLLOUT;
-       *runtime·netpolluser(pd) = (void*)events;
-
-       return runtime·port_associate(portfd, PORT_SOURCE_FD, fd, events, (uintptr)pd);
+       int32 r;
+
+       runtime·netpolllock(pd);
+       // We don't register for any specific type of events yet, that's
+       // netpollarm's job. We merely ensure we call port_associate before
+       // asynchonous connect/accept completes, so when we actually want
+       // to do any I/O, the call to port_associate (from netpollarm,
+       // with the interested event set) will unblock port_getn right away
+       // because of the I/O readiness notification.
+       *runtime·netpolluser(pd) = 0;
+       r = runtime·port_associate(portfd, PORT_SOURCE_FD, fd, 0, (uintptr)pd);
+       runtime·netpollunlock(pd);
+       return r;
 }
 
 int32
@@ -83,6 +153,9 @@ runtime·netpollclose(uintptr fd)
        return runtime·port_dissociate(portfd, PORT_SOURCE_FD, fd);
 }
 
+// Updates the association with a new set of interested events. After
+// this call, port_getn will return one and only one event for that
+// particular descriptor, so this function needs to be called again.
 void
 runtime·netpollupdate(PollDesc* pd, uint32 set, uint32 clear)
 {
@@ -90,22 +163,26 @@ runtime·netpollupdate(PollDesc* pd, uint32 set, uint32 clear)
        uintptr fd = runtime·netpollfd(pd);
        ep = (uint32*)runtime·netpolluser(pd);
 
-       do {
-               old = *ep;
-               events = (old & ~clear) | set;
-               if(old == events)
-                       return;
+       if(runtime·netpollclosing(pd))
+               return;
 
-               if(events && runtime·port_associate(portfd, PORT_SOURCE_FD, fd, events, (uintptr)pd) != 0) {
-                       runtime·printf("netpollupdate: failed to associate (%d)\n", errno);
-                       runtime·throw("netpollupdate: failed to associate");
-               }
-       } while(runtime·cas(ep, old, events) != events);
+       old = *ep;
+       events = (old & ~clear) | set;
+       if(old == events)
+               return;
+
+       if(events && runtime·port_associate(portfd, PORT_SOURCE_FD, fd, events, (uintptr)pd) != 0) {
+               runtime·printf("netpollupdate: failed to associate (%d)\n", errno);
+               runtime·throw("netpollupdate: failed to associate");
+       } 
+       *ep = events;
 }
 
+// subscribe the fd to the port such that port_getn will return one event.
 void
 runtime·netpollarm(PollDesc* pd, int32 mode)
 {
+       runtime·netpolllock(pd);
        switch(mode) {
        case 'r':
                runtime·netpollupdate(pd, POLLIN, 0);
@@ -116,6 +193,7 @@ runtime·netpollarm(PollDesc* pd, int32 mode)
        default:
                runtime·throw("netpollarm: bad mode");
        }
+       runtime·netpollunlock(pd);
 }
 
 // polls for ready network connections
@@ -126,7 +204,7 @@ runtime·netpoll(bool block)
        static int32 lasterr;
        PortEvent events[128], *ev;
        PollDesc *pd;
-       int32 i, mode;
+       int32 i, mode, clear;
        uint32 n;
        Timespec *wait = nil, zero;
        G *gp;
@@ -142,41 +220,43 @@ runtime·netpoll(bool block)
 
 retry:
        n = 1;
-
        if(runtime·port_getn(portfd, events, nelem(events), &n, wait) < 0) {
                if(errno != EINTR && errno != lasterr) {
                        lasterr = errno;
-                       runtime·printf("runtime: port_getn on fd %d "
-                           "failed with %d\n", portfd, errno);
+                       runtime·printf("runtime: port_getn on fd %d failed with %d\n", portfd, errno);
                }
                goto retry;
        }
 
        gp = nil;
-
        for(i = 0; i < n; i++) {
                ev = &events[i];
 
                if(ev->portev_events == 0)
                        continue;
-
-               if((pd = (PollDesc *)ev->portev_user) == nil)
-                       continue;
+               pd = (PollDesc *)ev->portev_user;
 
                mode = 0;
-
-               if(ev->portev_events & (POLLIN|POLLHUP|POLLERR))
+               clear = 0;
+               if(ev->portev_events & (POLLIN|POLLHUP|POLLERR)) {
                        mode += 'r';
-
-               if(ev->portev_events & (POLLOUT|POLLHUP|POLLERR))
+                       clear |= POLLIN;
+               }
+               if(ev->portev_events & (POLLOUT|POLLHUP|POLLERR)) {
                        mode += 'w';
-
-               //
+                       clear |= POLLOUT;
+               }
                // To effect edge-triggered events, we need to be sure to
                // update our association with whatever events were not
-               // set with the event.
-               //
-               runtime·netpollupdate(pd, 0, ev->portev_events & (POLLIN|POLLOUT));
+               // set with the event. For example if we are registered
+               // for POLLIN|POLLOUT, and we get POLLIN, besides waking
+               // the goroutine interested in POLLIN we have to not forget
+               // about the one interested in POLLOUT.
+               if(clear != 0) {
+                       runtime·netpolllock(pd);
+                       runtime·netpollupdate(pd, 0, clear);
+                       runtime·netpollunlock(pd);
+               }
 
                if(mode)
                        runtime·netpollready(&gp, pd, mode);
index 01294b70a05e73143f3553e765642e7b44b15fb0..baa751cd72ba819d9af991972dc7dad4d50db5a7 100644 (file)
@@ -965,6 +965,9 @@ void        runtime·netpollready(G**, PollDesc*, int32);
 uintptr        runtime·netpollfd(PollDesc*);
 void   runtime·netpollarm(PollDesc*, int32);
 void** runtime·netpolluser(PollDesc*);
+bool   runtime·netpollclosing(PollDesc*);
+void   runtime·netpolllock(PollDesc*);
+void   runtime·netpollunlock(PollDesc*);
 void   runtime·crash(void);
 void   runtime·parsedebugvars(void);
 void   _rt0_go(void);