Smalltalk › Squeak › Squeak - Dev

Socket>>#waitForDataFor:ifClosed:ifTimedOut: large delay hangs process

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

9 messages Options

squeakdev1

Socket>>#waitForDataFor:ifClosed:ifTimedOut: large delay hangs process

On rare occasions I've seen
Socket>>#waitForDataFor:ifClosed:ifTimedOut: (3.9 svp 7/27/2003 00:16)
pass a very large duration to readSemaphore waitTimeoutMSecs:. Large
enough to either hang the process for days, or cause Delay to complain
that delays cant exceed ~6 days. (when the duration should have been a
max of 300 seconds).

Is it possible that there is a problem in this code when the
millisecondClockValue is reset or rolls over? I'm was wondering if
totalSeconds could be used instead of milliseconds to avoid roll
overs, or is seconds too coarse to be used here?

Tom Phoenix

Re: Socket>>#waitForDataFor:ifClosed:ifTimedOut: large delay hangs process

On Jan 14, 2008 7:55 AM, <[hidden email]> wrote:

> On rare occasions I've seen
> Socket>>#waitForDataFor:ifClosed:ifTimedOut: (3.9 svp 7/27/2003 00:16)
> pass a very large duration to readSemaphore waitTimeoutMSecs:. Large
> enough to either hang the process for days, or cause Delay to complain
> that delays cant exceed ~6 days. (when the duration should have been a
> max of 300 seconds).
>
> Is it possible that there is a problem in this code when the
> millisecondClockValue is reset or rolls over?

It certainly smells like that, doesn't it? The code in
Socket>>#waitFor... doesn't show any awareness that the millisecond
clock can roll over. If the deadline falls just after a rollover, then
once the clock went back to zero, that method would ask for a delay
that lasts until after the *next* rollover. I think you've nailed it.

You should file a bug report for this on Mantis, if there isn't one already.

http://bugs.squeak.org

Cheers!

--Tom Phoenix

keith1y

Re: Socket>>#waitForDataFor:ifClosed:ifTimedOut: large delay hangs process

Tom Phoenix wrote:

> On Jan 14, 2008 7:55 AM, <[hidden email]> wrote:
>
>
>> On rare occasions I've seen
>> Socket>>#waitForDataFor:ifClosed:ifTimedOut: (3.9 svp 7/27/2003 00:16)
>> pass a very large duration to readSemaphore waitTimeoutMSecs:. Large
>> enough to either hang the process for days, or cause Delay to complain
>> that delays cant exceed ~6 days. (when the duration should have been a
>> max of 300 seconds).
>>
>> Is it possible that there is a problem in this code when the
>> millisecondClockValue is reset or rolls over?
>>
>
> It certainly smells like that, doesn't it? The code in
> Socket>>#waitFor... doesn't show any awareness that the millisecond
> clock can roll over. If the deadline falls just after a rollover, then
> once the clock went back to zero, that method would ask for a delay
> that lasts until after the *next* rollover. I think you've nailed it.
>
> You should file a bug report for this on Mantis, if there isn't one already.
>
> http://bugs.squeak.org
>
> Cheers!
>
> --Tom Phoenix

In 3.10 there is a timeout calculating function which does take roll
over into account.

Keith

squeakdev1

Re: Socket>>#waitForDataFor:ifClosed:ifTimedOut: large delay hangs process

In reply to this post by squeakdev1

,> In 3.10 there is a timeout calculating function which does take roll
over into account.

Can you tell me where that is? I checked 3.10 gamma.7159, the latest
3.10 image I could find, and the code for
Socket>>#waitForDataFor:ifClosed:ifTimedOut is the same 07/272003
00:16 version.

Tom Phoenix

Re: Socket>>#waitForDataFor:ifClosed:ifTimedOut: large delay hangs process

On Jan 14, 2008 12:58 PM, <[hidden email]> wrote:

> ,> In 3.10 there is a timeout calculating function which does take roll
> over into account.
>
> Can you tell me where that is?

I don't know about that one, but here's the fix I've put into the
method in my own image, for what it's worth. Just add the variable
waitFor at the top of the method, and then at the right place:

waitFor := deadline - Time millisecondClockValue.
waitFor > 300000 "300 seconds = 5 minutes"
ifTrue: [
"msec clock rolled over"
deadline := 0]
ifFalse: [
self readSemaphore waitTimeoutMSecs: waitFor] ].

This code handles the clock rollover somewhat more gracefully. Good
luck with it!

--Tom Phoenix

squeakdev1

Re: Socket>>#waitForDataFor:ifClosed:ifTimedOut: large delay hangs process

In reply to this post by squeakdev1

Tom, thanks for the code. I'm wondering what the consequence might be
to not waiting at all, when the clock rolls overs, as opposed to
waiting the specified time.

But my bigger concern, is that Time class>>#millisecondClockValue is
used in several other places that also do not take rollover into
account. Eg

Socket>>waitForDisconnectionFor:
Socket>>#waitForSendDoneFor
Socket class>>#deadlineSecs:
and in 3.10
Time class >> #deadlineSecs:

and others, no doubt.

Maybe an absolute millisecond clock value could be used instead of
Time millisecondClockValue, thereby abstracting the handling of
rollover into one place. A simple implementation would be:

(Time millisecondsSince:0)

Anyone see any problem with that?

Tom Phoenix

Re: Socket>>#waitForDataFor:ifClosed:ifTimedOut: large delay hangs process

On Jan 14, 2008 11:06 PM, <[hidden email]> wrote:

> Maybe an absolute millisecond clock value could be used instead of
> Time millisecondClockValue, thereby abstracting the handling of
> rollover into one place.

Maybe that's what should have been done in the first place, at least
in part because it's so hard to debug such rarely-used code.

Cheers!

--Tom Phoenix

squeakdev1

Re: Socket>>#waitForDataFor:ifClosed:ifTimedOut: large delay hangs process

In reply to this post by squeakdev1

The trick is to come up with a robust implementation of
absoluteMillisecondClockValue. My previous idea, (Time
millisecondsSince:0) was wrong; it is equivalent to just (Time
millisecondClockValue). Maybe something like:

absoluteMillisecondClockValue

| last next wrap |
wrap := SmallInteger maxVal. "is this correct?"
last := LastAbsoluteMillisecondClockValue ifNil:[0]. "class var"
next := Time millisecondClockValue .
[next < last ] whileTrue:[ next := next + wrap].
"or:
next < last ifTrue:[ next := last + (wrap - (last \\ wrap)) + next]."
^LastAbsoluteMillisecondClockValue := next

> Maybe that's what should have been done in the first place, at least
> in part because it's so hard to debug such rarely-used code.
>
> Cheers!
>
> --Tom Phoenix
>

David T. Lewis

Re: Socket>>#waitForDataFor:ifClosed:ifTimedOut: large delay hangs process

On Tue, Jan 15, 2008 at 01:54:29PM -0500, [hidden email] wrote:

> The trick is to come up with a robust implementation of
> absoluteMillisecondClockValue. My previous idea, (Time
> millisecondsSince:0) was wrong; it is equivalent to just (Time
> millisecondClockValue). Maybe something like:
>
> absoluteMillisecondClockValue
>
> | last next wrap |
> wrap := SmallInteger maxVal. "is this correct?"
> last := LastAbsoluteMillisecondClockValue ifNil:[0]. "class var"
> next := Time millisecondClockValue .
> [next < last ] whileTrue:[ next := next + wrap].
> "or:
> next < last ifTrue:[ next := last + (wrap - (last \\ wrap)) + next]."
> ^LastAbsoluteMillisecondClockValue := next

No, nothing like this is going to work reliably. Squeak VMs report clock
values converted to local time, which means that anything you come up
with is going to break horribly on daylight savings transitions. Furthermore,
the clock rollover rules are not necessarily the same on different VMs and
operating systems. The solution (a primitive reporting UTC ticks plus DST
offset) is straightforward and was proposed by Lex Spoon many years ago,
but never attracted much interest.

Dave