RFR 8231187: SelectorProvider.inheritedChannel() returns TCP socket channel for Unix domain socket

[Chris Hegarty]

Michael,

> On 23 Sep 2019, at 19:04, Michael McMahon <michael.x.mcmahon at oracle.com> wrote:
> 
> On 23/09/2019, 18:32, Alan Bateman wrote:
>> On 23/09/2019 17:56, Michael McMahon wrote:
>>> Okay, I see the reason for it now. It is the potential race when closing a socket
>>> and its file descriptor possibly being reassigned to another opened file, at the same time
>>> as another thread starting an I/O operation on the original socket.
>>> 
>>> I have added the two stage close logic similar to SocketChannel and the Source/SinkChannels you
>>> mentioned.
>>> 
>>> Updated webrev at: http://cr.openjdk.java.net/~michaelm/8231187/webrev.2/index.html


This looks good. A few comments..

  - I only see one usage of private UnixDomainSocketChannelImpl::getFD
  accessor, while other usages deference the field directly. Can getFD
  be removed, or why is that usage different?

  - I'm a little confused by the following, in UnixDomainSocketChannelImpl:

    156             } finally {
    157                 endRead(n > 0);
    158                 if (n <= 0)
    159                     return IOStatus.EOF;
    160             }
    161             return IOStatus.normalize(n);

    Is it always the case that if `n` is less than or equal to zero,
    that the socket is at EOF? what about errors?  If the former is
    true, then there is no need to normalize `n`, it is guaranteed to be
    positive, no?

  - In UnixDomainSocketChannelImpl(..)

     69     UnixDomainSocketChannelImpl(FileDescriptor fd)
     70         throws IOException
     71     {
     72         this.fd = fd;
     73         this.state = ST_INUSE;
     74     }

   The setting of the state here is, without holding the stateLock,
   raises questions. But it should be safe since ST_INUSE is the
   primitive's default initial value, `0`. If that were to change, then
   not holding the lock could be problematic. Maybe just grab the lock.

  - In UnixDomainSocketChannelImpl:

   170      * @throws ClosedChannelException if the channel is closed or output shutdown

   output cannot be shutdown - please remove "or output shutdown"


  - Maybe update the copyright year ranges in the existing files
  
-Chris.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20190924/2abf2137/attachment.html>