Status: FixedWaitingToBePharoed Owner: stephane.ducasse Labels: Milestone-1.3 New issue 3346 by stephane.ducasse: Socket bug fixes http://code.google.com/p/pharo/issues/detail?id=3346 Andreas Raab uploaded a new version of Network to project The Trunk: http://source.squeak.org/trunk/Network-ar.98.mcz ==================== Summary ==================== Name: Network-ar.98 Author: ar Time: 23 November 2010, 12:12:58.498 am UUID: 5f0637f3-9b53-ab42-a6fa-55077db31f63 Ancestors: Network-ul.97 Fixes for SocketStream: 1) SocketStream>>receiveData: and SocketStream>>upToEnd: had incorrect conditions used to determine when to stop (#isConnected is wrong since there can be data pending on an unconnected socket which is why using #atEnd is the correct test). 2) The former was done to deal with non-signaling SocketStreams in cases where ConnectionClose was used to deal with end conditions (#upToEnd, #next:into:startingAt:, #readInto:startingAt:count:). This was fixed by making these places (temporarily) signaling so that the exception can be caught and handled properly. 3) SocketStream>>isDataAvailable should not attempt to shortcut prematurely; using 'socket dataAvailable' is bad for subclasses such as SecureSocketStream and unnecessary to boot (and has no performance impact). =============== Diff against Network-ul.97 =============== Item was added: + ----- Method: SocketStream>>beSignalingWhile: (in category 'private') ----- + beSignalingWhile: aBlock + "Temporarily turn a non-signaling SocketStream into a signaling one. + Required for some of operations that will catch ConnectionClosed in + order to find out that an operation completed" + + | signaling | + signaling := shouldSignal. + shouldSignal := true. + ^aBlock ensure:[shouldSignal := signaling] + ! Item was changed: ----- Method: SocketStream>>isDataAvailable (in category 'testing') ----- isDataAvailable + "Answer if more data can be read. It the inbuffer is empty, we read more data. - "Answer if more data can be read. It the inbuffer is empty, we check the socket for data. If it claims to have data available to read, we try to read some once and recursively call this method again. If something really was available it is now in the inBuffer. This is because there has been spurious dataAvailable when there really is no data to get. + Note: It is important not to rely on 'socket dataAvailable' here since this will + not work for subclasses such as SecureSocketStream (which can contain + undecrypted contents that has been read from the socket)." - Note: Some subclasses (such as SecureSocketStream) rely on the behavior here since even though data may be available in the underlying socket, it may not result in any output (yet)." self isInBufferEmpty ifFalse: [^true]. + ^self receiveAvailableData < inNextToWrite + ! - ^socket dataAvailable - ifFalse: [false] - ifTrue: [self receiveAvailableData; isDataAvailable]! Item was changed: ----- Method: SocketStream>>next:into:startingAt: (in category 'stream in') ----- next: anInteger into: aCollection startingAt: startIndex "Read n objects into the given collection. Return aCollection or a partial copy if less than n elements have been read." "Implementation note: This method DOES signal timeout if not enough elements are received. It does NOT signal ConnectionClosed as closing the connection is the only way by which partial data can be read." | start amount | + [self beSignalingWhile:[self receiveData: anInteger]] + on: ConnectionClosed do:[:ex| ex return]. - [self receiveData: anInteger] on: ConnectionClosed do:[:ex| ex return]. "Inlined version of nextInBuffer: to avoid copying the contents" amount := anInteger min: (inNextToWrite - lastRead - 1). start := lastRead + 1. lastRead := lastRead + amount. aCollection replaceFrom: startIndex to: startIndex + amount-1 with: inBuffer startingAt: start. ^amount < anInteger ifTrue:[aCollection copyFrom: 1 to: startIndex + amount-1] ifFalse:[aCollection]! Item was changed: ----- Method: SocketStream>>readInto:startingAt:count: (in category 'stream in') ----- readInto: aCollection startingAt: startIndex count: anInteger "Read n objects into the given collection starting at startIndex. Return number of elements that have been read." "Implementation note: This method DOES signal timeout if not enough elements are received. It does NOT signal ConnectionClosed as closing the connection is the only way by which partial data can be read." | start amount | + [self beSignalingWhile:[self receiveData: anInteger]] + on: ConnectionClosed do:[:ex| ex return]. - [self receiveData: anInteger] on: ConnectionClosed do:[:ex| ex return]. "Inlined version of nextInBuffer: to avoid copying the contents" amount := anInteger min: (inNextToWrite - lastRead - 1). start := lastRead + 1. lastRead := lastRead + amount. aCollection replaceFrom: startIndex to: startIndex + amount-1 with: inBuffer startingAt: start. ^amount! Item was changed: ----- Method: SocketStream>>receiveData: (in category 'control') ----- receiveData: nBytes "Keep reading the socket until we have nBytes in the inBuffer or we reach the end. This method does not return data, but can be used to make sure data has been read into the buffer from the Socket before actually reading it from the FastSocketStream. Mainly used internally. We could also adjust the buffer to the expected amount of data and avoiding several incremental grow operations. NOTE: This method doesn't honor timeouts if shouldSignal is false!! And frankly, I am not sure how to handle that case or if I care - I think we should always signal." + [self atEnd not and: [nBytes > self inBufferSize]] - [self isConnected and: [nBytes > self inBufferSize]] whileTrue: [self receiveData]! Item was changed: ----- Method: SocketStream>>upToEnd (in category 'stream in') ----- upToEnd "Answer all data coming in on the socket until the socket is closed by the other end, or we get a timeout. + This means this method catches ConnectionClosed by itself." - This means this method catches ConnectionClosed by itself. - - NOTE: Does not honour timeouts if shouldSignal is false!!" + [[self atEnd] whileFalse: [self beSignalingWhile:[self receiveData]]] - [[self isConnected] whileTrue: [self receiveData]] on: ConnectionClosed do: [:ex | "swallow it"]. ^self nextAllInBuffer!