>>#next: SocketStream and other oddities
Hi, while writing some SMS PDU parsing code I stumbled across an "oddity" I have not seen with GNU Smalltalk. When calling the
#next: selector on a ReadStream and the number of elements I need are not available, Pharo will return less or an empty collection. This means I write code like this:
| id len | id := stream next: 4 id size = 4 ifFalse: [^self reportError: 'Short read']. len := stream next: 4 len size = 4 ifFalse: [^self reportError: 'Len not available']. Besides the aesthetic issue it is getting worse when the SocketStream class is in use. So now >>#next: 4 can return nil as well. But naively I wrote code like this: [ | msg | msg := ThingParser parse: aSocketStream. ] on: ConnectionClosed do: [:e | self logStuff: e. self scheduleReconnect ] So to make the above code work with the SocketStream I need to write the following. I had a quick look at why the ConnectionClosed is swallowed and found the comment to allow partial reads out of a dead socket. | id len | id := stream next: 4. id ifNil: [^self reportError: 'Short read']. len := stream next: 4 len ifNil: [^self reportError: 'Short read']. len size = 4 ifFalse: [^self reportError: 'Len not available'] In GNU Smalltalk there is EndOfStream which is a subclass of Notification and the above code boils down to: [ | id len | id := stream next: 4. len := stream next: 4. ] on: EndOfStream do: [:e | self logStuff: e. self scheduleReconnect. ]. So couldn't the >>#readInto:startingAt:count: selector of SocketStream do something like GNU Smalltalk and give an EndOfStream notification? [self receiveData: anInteger] on: ConnectionClosed do:[:ex| EndOfStream signalOn: self. ex return ]. kind regards holger
Hi holger I got confused. There is a problem with next: not raising error? Stef On 29/8/14 09:16, Holger Hans Peter Freyther wrote:
Hi,
while writing some SMS PDU parsing code I stumbled across an "oddity" I have not seen with GNU Smalltalk. When calling the
#next: selector on a ReadStream and the number of elements I need are not available, Pharo will return less or an empty collection. This means I write code like this:
| id len | id := stream next: 4 id size = 4 ifFalse: [^self reportError: 'Short read'].
len := stream next: 4 len size = 4 ifFalse: [^self reportError: 'Len not available'].
Besides the aesthetic issue it is getting worse when the SocketStream class is in use. So now >>#next: 4 can return nil as well. But naively I wrote code like this:
[ | msg | msg := ThingParser parse: aSocketStream. ] on: ConnectionClosed do: [:e | self logStuff: e. self scheduleReconnect ]
So to make the above code work with the SocketStream I need to write the following. I had a quick look at why the ConnectionClosed is swallowed and found the comment to allow partial reads out of a dead socket.
| id len | id := stream next: 4. id ifNil: [^self reportError: 'Short read'].
len := stream next: 4 len ifNil: [^self reportError: 'Short read']. len size = 4 ifFalse: [^self reportError: 'Len not available']
In GNU Smalltalk there is EndOfStream which is a subclass of Notification and the above code boils down to:
[ | id len | id := stream next: 4. len := stream next: 4. ] on: EndOfStream do: [:e | self logStuff: e. self scheduleReconnect. ].
So couldn't the >>#readInto:startingAt:count: selector of SocketStream do something like GNU Smalltalk and give an EndOfStream notification?
[self receiveData: anInteger] on: ConnectionClosed do:[:ex| EndOfStream signalOn: self. ex return ].
kind regards holger
On Sat, Aug 30, 2014 at 09:19:49AM +0200, stepharo wrote:
Hi holger
Hi! thanks for the answer.
I got confused.
sorry about that.
There is a problem with next: not raising error?
Classic ReadStream in Pharo3.0 #[] readStream next: 2 => #[] SocketStream: 1.) Start something that listens to 12345 $ nc -l -p 12345 2.) In Pharo do | conn | conn := (SocketStream openConnectionToHostNamed: 'localhost' port: 12345) binary; noTimeout; yourself. [ (conn next: 4) inspect. ] on: ConnectionClosed do: [:e | "Never called" self halt ]. 3.) CTRL+C the netcat process.. 4.) In Pharo an inspector on an ByteArray is coming up. The ConnectionClosed signal is raised but swallowed by the SocketStream code. I don't know how realistic it is but I would appreciate if 1.) #[] readStream next: 4. Could raise an Error that one can not consume four bytes/elements. 2.) I understand that not raising "ConnectionClosed" is to allow a partial read from the socket. What is done in GNU Smalltalk is to at least signal an EndOfStream notificaton. So the socket code could read like: | conn | conn := (SocketStream openConnectionToHostNamed: 'localhost' port: 12345) binary; noTimeout; yourself. [ (conn next: 4) inspect. ] on: EndOfStream do: [:e | self handleClosing ]. is this more clear? holger
I'm not an expert on networking stuff, but I'll try (and if I'm wrong I'll learn something new)... Holger Hans Peter Freyther wrote:
Classic ReadStream in Pharo3.0
#[] readStream next: 2 => #[]
SocketStream:
1.) Start something that listens to 12345 $ nc -l -p 12345
2.) In Pharo do
| conn | conn := (SocketStream openConnectionToHostNamed: 'localhost' port: 12345) binary; noTimeout; yourself. [ (conn next: 4) inspect. ] on: ConnectionClosed do: [:e | "Never called" self halt ].
3.) CTRL+C the netcat process.. 4.) In Pharo an inspector on an ByteArray is coming up. The ConnectionClosed signal is raised but swallowed by the SocketStream code.
I don't know how realistic it is but I would appreciate if
1.) #[] readStream next: 4. Could raise an Error that one can not consume four bytes/elements.
Making it an Error seems a somewhat fundamental change that could break other people's code. Would a Notification be sufficient?
2.) I understand that not raising "ConnectionClosed" is to allow a partial read from the socket. What is done in GNU Smalltalk is to at least signal an EndOfStream notificaton. So the socket code could read like:
Are you implying that in the same situation GNU Smalltalk similarly does not raise ConnectionClosed, but raises EndOfStream instead? EndOfStream as a Notification seems a low impact addition that would be generally useful. Can you determine the code changes required for SocketStream? The fastest way to get what you need is to open a ticket at pharo.fogbugz.com, then also supply the required code** as a slice :). This will provide something concrete to review. **Including tests, one test that catches your new Notification, and then a duplicate of that test that does not catch the Notification, which works the same before and after slice is applied. cheers -ben
| conn | conn := (SocketStream openConnectionToHostNamed: 'localhost' port: 12345) binary; noTimeout; yourself. [ (conn next: 4) inspect. ] on: EndOfStream do: [:e | self handleClosing ].
is this more clear?
holger
Holger, You are right and wrong ;-) SocketStream is what it is, it is far from perfect, but it works and had been working for a very long time. There are alternatives. The well documented contract of SocketStream #next: and all its variants up to #readInto:startingAt:count: states and explains the behaviour you describe. In other words, it is by design. I don't think these calls can return nil though. That is a behaviour of #next (also far from perfect). The semantics of SocketStream are not completely identical to ReadStream in general, because so much more can go wrong. Working with an explicit EOF exception is another design approach that has pros and cons as well. An alternative is Xtreams, which can be loaded into Pharo. Here, the API is much cleaner. Sven On 30 Aug 2014, at 11:04, Ben Coman <btc@openInWorld.com> wrote:
I'm not an expert on networking stuff, but I'll try (and if I'm wrong I'll learn something new)...
Holger Hans Peter Freyther wrote:
Classic ReadStream in Pharo3.0
#[] readStream next: 2 => #[]
SocketStream:
1.) Start something that listens to 12345 $ nc -l -p 12345
2.) In Pharo do
| conn | conn := (SocketStream openConnectionToHostNamed: 'localhost' port: 12345) binary; noTimeout; yourself. [ (conn next: 4) inspect. ] on: ConnectionClosed do: [:e | "Never called" self halt ].
3.) CTRL+C the netcat process.. 4.) In Pharo an inspector on an ByteArray is coming up. The ConnectionClosed signal is raised but swallowed by the SocketStream code.
I don't know how realistic it is but I would appreciate if
1.) #[] readStream next: 4. Could raise an Error that one can not consume four bytes/elements.
Making it an Error seems a somewhat fundamental change that could break other people's code. Would a Notification be sufficient?
2.) I understand that not raising "ConnectionClosed" is to allow a partial read from the socket. What is done in GNU Smalltalk is to at least signal an EndOfStream notificaton. So the socket code could read like:
Are you implying that in the same situation GNU Smalltalk similarly does not raise ConnectionClosed, but raises EndOfStream instead? EndOfStream as a Notification seems a low impact addition that would be generally useful. Can you determine the code changes required for SocketStream? The fastest way to get what you need is to open a ticket at pharo.fogbugz.com, then also supply the required code** as a slice :). This will provide something concrete to review. **Including tests, one test that catches your new Notification, and then a duplicate of that test that does not catch the Notification, which works the same before and after slice is applied.
cheers -ben
| conn | conn := (SocketStream openConnectionToHostNamed: 'localhost' port: 12345) binary; noTimeout; yourself. [ (conn next: 4) inspect. ] on: EndOfStream do: [:e | self handleClosing ].
is this more clear?
holger
participants (4)
-
Ben Coman -
Holger Hans Peter Freyther -
stepharo -
Sven Van Caekenberghe