netFD.Close()

[ron minnich]

I was having a problem with the gproc code (gproc is a
reimplementation of LANL Bproc in Go) and found the following:

func (fd *netFD) Close() os.Error {
if fd == nil || fd.sysfile == nil {
return os.EINVAL
}

fd.incref()
syscall.Shutdown(fd.sysfd, syscall.SHUT_RDWR)
fd.closing = true
fd.decref()
return nil
}

That shutdown looks to me like a mistake. The problem is that shutdown
more or less goes "underneath" the fd to shut down the network
connection. The fd is still there, but no I/O will work.

If the netFD is shared, then the Shutdown is going to make the netFD
totally unusable to every other goroutine, even though the netFD, and
the underlying fd, are not closed (the reference counting ensures
close will not be called until decref hits 0). Could that really be
the intent of this code? I can't see any use for this behavior.

Still worse, if the underlying fd is is shared via a Dup, Dup2, or
Exec, then the Shutdown is going to blow away the connection, even as
the other process or user expects their copy of the fd to be
functional.

I'm thinking of submitting a patch for this code, which simply removes
the call to Shutdown, but I'm wondering if there is some rationale
here that I'm missing. Right on the face of it this code seems wrong,
however.

Thanks

ron
p.s. FWIW, it is starting to look like gproc can do in 500 lines (the
test is 200 lines now) what we did in C in 12,000 lines. This assuming
some of my elf and net patchs get accepted.

[Ian Lance Taylor]

ron minnich <rmin...@gmail.com> writes:

The sthudown is intentional. Once the netFD is closed by any goroutine,
it is no longer available to any other goroutine. The file descriptor
is not actually closed at this point to avoid a race condition with the
poll server. That's the reason for the reference count.

The point about Dup and particularly Exec is a good one, though. I
think the way the file descriptor is set to be nonblocking before
calling close in decref will also mess that up. It's probably fine to
skip the call to shutdown and check fd.closing in Read and friends. I'm
not sure what to do about the call to SetNonblock in decref, though.

Ian