[squeak-dev] Setting a preempted process priority glitch

> Here the test, showing that setting a priority of preempted process
> may not result of what you expected:
>
> | first p1 p2 |
> first := nil.
> p1 := [ first ifNil: [ first:= 1] ] forkAt: Processor activePriority - 1.
> p2 := [ first ifNil: [ first:= 2] ] forkAt: Processor activePriority - 2.
> p1 priority: 10.
> (Delay forMilliseconds: 100) wait.
> first
>
> at first, an order in which processes should awake is p1 then p2.
> But then, if we setting p1 priority lower than p2 , the order should
> be p2 then p1.
>
> To fix this we should reschedule preempted process:
>
> Process>>priority: anInteger
> (anInteger >= Processor lowestPriority and:[anInteger <= Processor
> highestPriority])
> ifTrue: [priority := anInteger]

> On Fri, 01 May 2009 11:14:02 +0200, Igor Stasenko wrote:
>
>> Here the test, showing that setting a priority of preempted process
>> may not result of what you expected:
>>
>> | first p1 p2 |
>> first := nil.
>> p1 := [ first ifNil: [ first:= 1] ] forkAt: Processor activePriority - 1.
>> p2 := [ first ifNil: [ first:= 2] ] forkAt: Processor activePriority - 2.
>> p1 priority: 10.
>> (Delay forMilliseconds: 100) wait.
>> first
>>
>> at first, an order in which processes should awake is p1 then p2.
>> But then, if we setting p1 priority lower than p2 , the order should
>> be p2 then p1.
>>
>> To fix this we should reschedule preempted process:
>>
>> Process>>priority: anInteger
>> (anInteger >= Processor lowestPriority and:[anInteger <= Processor
>> highestPriority])
>>        ifTrue: [priority := anInteger]
>
> Awesome, this block with assigning priority together with [^ self] below :)
> if scheduler ever sees this guy again it will DoTheRightThing :)

>> This issue shows, that comment in Process>>#terminate, lying to us:
>>
>> "Since the receiver is not the active process, drop its priority to
>> rock-bottom so that
>
> :(
>
>> it doesn't accidentally preempt the process that is trying to terminate
>> it."
>> priority := 10.
>
>
>

> Igor Stasenko wrote:
>>
>> This issue shows, that comment in Process>>#terminate, lying to us:
>>
>> "Since the receiver is not the active process, drop its priority to
>> rock-bottom so that
>> it doesn't accidentally preempt the process that is trying to terminate
>> it."
>> priority := 10.
>
> It's actually saying the right thing. What this code is trying to avoid is
> that a process waiting on some semaphore (like a network thread) starts
> "running away" when it receives a signal (we've seen this happening in
> several cases). Contrary to your example the terminating process has no wait
> during which the terminated process could run at lower priority. And if the
> terminated process were at higher priority and not currently waiting, then
> obviously the terminating process would not execute. So it all works out.
>

> But the real fix for these issues is atomic suspend, i.e., instead of:
>
>        priority := 10.
>        myList ifNotNil: [
>                myList remove: self ifAbsent: [].
>                "Figure out if the receiver was terminated while waiting on a
> Semaphore"
>                inSema := myList class == Semaphore.
>                myList := nil].
>
>
> this needs to be:
>
>        oldList := self suspend.
>        inSema := oldList class == Semaphore.
>

> Here the test, showing that setting a priority of preempted process
> may not result of what you expected:
>
> | first p1 p2 |
> first := nil.
> p1 := [ first ifNil: [ first:= 1] ] forkAt: Processor activePriority - 1.
> p2 := [ first ifNil: [ first:= 2] ] forkAt: Processor activePriority - 2.
> p1 priority: 10.
> (Delay forMilliseconds: 100) wait.
> first
>
> at first, an order in which processes should awake is p1 then p2.
> But then, if we setting p1 priority lower than p2 , the order should
> be p2 then p1.
>
> To fix this we should reschedule preempted process:
>
> Process>>priority: anInteger
> (anInteger >= Processor lowestPriority and:[anInteger <= Processor
> highestPriority])
>        ifTrue: [priority := anInteger]
>        ifFalse: [self error: 'Invalid priority: ', anInteger printString]
> (myList isNil or: [myList class == Semaphore]) ifTrue: [ ^ self  ].
> "we are preempted. reschedule at new priority"
> self suspend;resume.
>
> This issue shows, that comment in Process>>#terminate, lying to us:
>
> "Since the receiver is not the active process, drop its priority to
> rock-bottom so that
> it doesn't accidentally preempt the process that is trying to terminate it."
> priority := 10.
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>
>

> 2009/5/1 Andreas Raab <[hidden email]>:
>> Igor Stasenko wrote:
>>>
>>> This issue shows, that comment in Process>>#terminate, lying to us:
>>>
>>> "Since the receiver is not the active process, drop its priority to
>>> rock-bottom so that
>>> it doesn't accidentally preempt the process that is trying to terminate
>>> it."
>>> priority := 10.
>>
>> It's actually saying the right thing. What this code is trying to avoid is
>> that a process waiting on some semaphore (like a network thread) starts
>> "running away" when it receives a signal (we've seen this happening in
>> several cases). Contrary to your example the terminating process has no wait
>> during which the terminated process could run at lower priority. And if the
>> terminated process were at higher priority and not currently waiting, then
>> obviously the terminating process would not execute. So it all works out.
>>
>
> if terminated process is at higher priority and not waiting then it:
> a) should run now , instead of process which tries to terminate it
> b) should be suspended
> but there are always a chance that:
> c) another, higher priority process interrupts current process and
> doing something which makes terminated process to run again.
>
> in this case, a terminated process can have any priority & state, it
> will not save you from other's process intrusion to do anything with
> it (its really interesting to see what will happen if two different
> processes decide to terminate same process, and preempt each other
> during unwinding phase ;-) ). That's, to my thinking, can be avoided
> only by introducing an additional ivar to process, to test its state.
>
> And even, if higher priority don't does anything with terminated
> process, try this:
>
> | x procToTerminate intruder sema |
> sema := Semaphore new.
> x := nil.
> procToTerminate := [ x:= 1] fork.
> intruder := [ sema wait ] forkAt: Processor activePriority +1.
> "now suppose here we started a termination procedure ... "
> procToTerminate priority: 10.
> " and somewhere in the middle we got external signal (such as from
> Delay or Socket) "
> sema signal.
>
> x isNil
>

>
>> But the real fix for these issues is atomic suspend, i.e., instead of:
>>
>>        priority := 10.
>>        myList ifNotNil: [
>>                myList remove: self ifAbsent: [].
>>                "Figure out if the receiver was terminated while waiting on a
>> Semaphore"
>>                inSema := myList class == Semaphore.
>>                myList := nil].
>>
>>
>> this needs to be:
>>
>>        oldList := self suspend.
>>        inSema := oldList class == Semaphore.
>>
>
> yes, but before that, i would insert:
>
> | canTerminate |
> Processor interruptWith: [
>   canTerminate := self isTerminating not.
>   self terminating: true.
> ].
> canTerminate ifTrue: [
>  .. unwind ..  blablabla
> ]
>
>> Cheers,
>>  - Andreas
>>
>>
>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>

> Julian
>
> On Fri, May 1, 2009 at 2:14 AM, Igor Stasenko <[hidden email]> wrote:
>> Here the test, showing that setting a priority of preempted process
>> may not result of what you expected:
>>
>> | first p1 p2 |
>> first := nil.
>> p1 := [ first ifNil: [ first:= 1] ] forkAt: Processor activePriority - 1.
>> p2 := [ first ifNil: [ first:= 2] ] forkAt: Processor activePriority - 2.
>> p1 priority: 10.
>> (Delay forMilliseconds: 100) wait.
>> first
>>
>> at first, an order in which processes should awake is p1 then p2.
>> But then, if we setting p1 priority lower than p2 , the order should
>> be p2 then p1.
>>
>> To fix this we should reschedule preempted process:
>>
>> Process>>priority: anInteger
>> (anInteger >= Processor lowestPriority and:[anInteger <= Processor
>> highestPriority])
>>        ifTrue: [priority := anInteger]
>>        ifFalse: [self error: 'Invalid priority: ', anInteger printString]
>> (myList isNil or: [myList class == Semaphore]) ifTrue: [ ^ self  ].
>> "we are preempted. reschedule at new priority"
>> self suspend;resume.
>>
>> This issue shows, that comment in Process>>#terminate, lying to us:
>>
>> "Since the receiver is not the active process, drop its priority to
>> rock-bottom so that
>> it doesn't accidentally preempt the process that is trying to terminate it."
>> priority := 10.
>>
>> --
>> Best regards,
>> Igor Stasenko AKA sig.
>>
>>
>
>

> 2009/5/1 Julian Fitzell <[hidden email]>:
>> Can I encourage you to submit unit tests for any bugs you find as well
>> so they don't regress even if you end up fixing the problem?
>>
>
> Certainly, i will.
>
>> Julian
>>
>> On Fri, May 1, 2009 at 2:14 AM, Igor Stasenko <[hidden email]> wrote:
>>> Here the test, showing that setting a priority of preempted process
>>> may not result of what you expected:
>>>
>>> | first p1 p2 |
>>> first := nil.
>>> p1 := [ first ifNil: [ first:= 1] ] forkAt: Processor activePriority - 1.
>>> p2 := [ first ifNil: [ first:= 2] ] forkAt: Processor activePriority - 2.
>>> p1 priority: 10.
>>> (Delay forMilliseconds: 100) wait.
>>> first
>>>
>>> at first, an order in which processes should awake is p1 then p2.
>>> But then, if we setting p1 priority lower than p2 , the order should
>>> be p2 then p1.
>>>
>>> To fix this we should reschedule preempted process:
>>>
>>> Process>>priority: anInteger
>>> (anInteger >= Processor lowestPriority and:[anInteger <= Processor
>>> highestPriority])
>>>        ifTrue: [priority := anInteger]
>>>        ifFalse: [self error: 'Invalid priority: ', anInteger printString]
>>> (myList isNil or: [myList class == Semaphore]) ifTrue: [ ^ self  ].
>>> "we are preempted. reschedule at new priority"
>>> self suspend;resume.
>>>
>>> This issue shows, that comment in Process>>#terminate, lying to us:
>>>
>>> "Since the receiver is not the active process, drop its priority to
>>> rock-bottom so that
>>> it doesn't accidentally preempt the process that is trying to terminate it."
>>> priority := 10.
>>>
>>> --
>>> Best regards,
>>> Igor Stasenko AKA sig.
>>>
>>>
>>
>>
>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>