The Trunk: Network-eem.172.mcz

> Hi All,
>     while the changes below are correct, in that they prevent primitive failures that shouldn't happen, they cause the testSocketReuse test to now lock up the system.  I;m trying to
> understand what's going on and develop a proper fix.  Levente, you allude to a VM bug in waitForDataIfClosed .  Can you be more precise?  It seems to me that the bug is that a socket's
> semaphore is not necessarily signalled when a socket becomes invalid, closes, etc.  Is that right?  I suppose that the primitive failures are preferrable to a lock up wuerh running the test
> suite, so perhaps we should revert the changes below.  But it feels wrong to mask a bug with a primitive failure.  It would be nicer to have something in the code that explicitly prevented
> getting into the lockup, even if by raising an error.
>
> On Thu, Feb 25, 2016 at 1:07 PM, <[hidden email]> wrote:
>       Eliot Miranda uploaded a new version of Network to project The Trunk:
>       http://source.squeak.org/trunk/Network-eem.172.mcz
>
>       ==================== Summary ====================
>
>       Name: Network-eem.172
>       Author: eem
>       Time: 25 February 2016, 1:07:33.303622 pm
>       UUID: 5c73783a-d4e2-413e-b5eb-f04ad8decee2
>       Ancestors: Network-ul.171
>
>       Fix two failures to check if a socket is closed (socketHandle is nil) before attempting to read from the socket.
>
>       =============== Diff against Network-ul.171 ===============
>
>       Item was changed:
>         ----- Method: Socket>>receiveDataInto:startingAt: (in category 'receiving') -----
>         receiveDataInto: aStringOrByteArray startingAt: aNumber
>               "Receive data into the given buffer and return the number of bytes received.
>               Note the given buffer may be only partially filled by the received data.
>               Waits for data once.  The answer may be zero (indicating that no data was
>               available before the socket closed)."
>
>       +       | bytesRead open |
>       -       | bytesRead closed |
>               bytesRead := 0.
>       +       open := true.
>       +       [open and: [bytesRead = 0]] whileTrue:
>       +               [self waitForDataIfClosed: [open := false].
>       +                open ifTrue:
>       +                       [bytesRead := self primSocket: socketHandle
>       +                                                               receiveDataInto: aStringOrByteArray
>       +                                                               startingAt: aNumber
>       +                                                               count: aStringOrByteArray size - aNumber + 1]].
>       -       closed := false.
>       -       [closed not and: [bytesRead = 0]]
>       -               whileTrue: [
>       -                       self waitForDataIfClosed: [closed := true].
>       -                       bytesRead := self primSocket: socketHandle
>       -                               receiveDataInto: aStringOrByteArray
>       -                               startingAt: aNumber
>       -                               count: aStringOrByteArray size-aNumber+1].
>               ^bytesRead
>         !
>
>       Item was changed:
>         ----- Method: Socket>>waitForDataIfClosed: (in category 'waiting') -----
>         waitForDataIfClosed: closedBlock
>               "Wait indefinitely for data to arrive.  This method will block until
>               data is available or the socket is closed."
>
>       +       [(socketHandle ~~ nil
>       +         and: [self primSocketReceiveDataAvailable: socketHandle]) ifTrue:
>       +               [^self].
>       +        self isConnected ifFalse:
>       +               [^closedBlock value].
>       +        "ul 8/13/2014 21:16
>       +         Providing a maximum for the time for waiting is a workaround for a VM bug which
>       +         causes sockets waiting for data forever in some rare cases, because the semaphore
>       +         doesn't get signaled. Replace the ""waitTimeoutMSecs: self class maximumReadSemaphoreWaitTimeout""
>       +         part with ""wait"" when the bug is fixed."
>       +        self readSemaphore waitTimeoutMSecs: self class maximumReadSemaphoreWaitTimeout] repeat!
>       -       [
>       -               (self primSocketReceiveDataAvailable: socketHandle)
>       -                       ifTrue: [^self].
>       -               self isConnected
>       -                       ifFalse: [^closedBlock value].
>       -               "Providing a maximum for the time for waiting is a workaround for a VM bug which causes sockets waiting for data forever in some rare cases, because the semaphore
>       doesn't get signaled. Replace the ""waitTimeoutMSecs: self class maximumReadSemaphoreWaitTimeout"" part with ""wait"" when the bug is fixed."
>       -               self readSemaphore waitTimeoutMSecs: self class maximumReadSemaphoreWaitTimeout ] repeat
>       - !
>
>
>
>
>
> --
> _,,,^..^,,,_
> best, Eliot
>
>