Hi all,
In the method Process>>resume, which invokes primitive 87, the comment states that the primitive should fail if the process is already waiting in a queue. But the implementation does not do that. If we execute (inspect) the snippet:
| evaluationProcess result delay testProcess | delay := Delay forMilliseconds: 50. testProcess := Processor activeProcess. evaluationProcess := [ delay unschedule. result := testProcess isBlocked. testProcess resume] fork. delay wait. result
it returns true (and no primitive failed error occurs), which means we successfully resumed a blocked (waiting on a semaphore) process. I also checked in VMMaker and indeed, the code does not check if the process is on a queue or not before just putting it on a runnable list.
What do you think? Should the comment be changed or the implementation?
Florin
Hi Florin,
On Wed, May 19, 2021 at 4:05 PM Florin Mateoc florin.mateoc@gmail.com wrote:
Hi all,
In the method Process>>resume, which invokes primitive 87, the comment states that the primitive should fail if the process is already waiting in a queue. But the implementation does not do that. If we execute (inspect) the snippet:
| evaluationProcess result delay testProcess | delay := Delay forMilliseconds: 50. testProcess := Processor activeProcess. evaluationProcess := [ delay unschedule. result := testProcess isBlocked. testProcess resume] fork. delay wait. result
it returns true (and no primitive failed error occurs), which means we successfully resumed a blocked (waiting on a semaphore) process. I also checked in VMMaker and indeed, the code does not check if the process is on a queue or not before just putting it on a runnable list.
What do you think? Should the comment be changed or the implementation?
I think the implementation. Any other opinions?
Florin
Hi Florin,
here's my comment in the implementation from 2010:
"Personally I would like to check MyList, which should not be one of the elements of the scheduler lists. But there are awful race conditions in things like should:notTakeMoreThan: that mean we can't. eem 9/27/2010 23:08. e.g.
So it appears that there may be issues that make it difficult to change the primitive.
On Wed, May 19, 2021 at 4:05 PM Florin Mateoc florin.mateoc@gmail.com wrote:
Hi all,
In the method Process>>resume, which invokes primitive 87, the comment states that the primitive should fail if the process is already waiting in a queue. But the implementation does not do that. If we execute (inspect) the snippet:
| evaluationProcess result delay testProcess | delay := Delay forMilliseconds: 50. testProcess := Processor activeProcess. evaluationProcess := [ delay unschedule. result := testProcess isBlocked. testProcess resume] fork. delay wait. result
it returns true (and no primitive failed error occurs), which means we successfully resumed a blocked (waiting on a semaphore) process. I also checked in VMMaker and indeed, the code does not check if the process is on a queue or not before just putting it on a runnable list.
What do you think? Should the comment be changed or the implementation?
Florin
Hi Florin,
I submitted a suitable implementation to VMMakerInbox for our records, see VMMaker.oscog-eem.2959. Some time I'll try and test this to see if the issues are serious or not.
On Thu, May 20, 2021 at 3:15 PM Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Florin,
here's my comment in the implementation from 2010:
"Personally I would like to check MyList, which should not be one of the elements of the scheduler lists. But there are awful race conditions in things like should:notTakeMoreThan: that mean we can't. eem 9/27/2010 23:08. e.g.
So it appears that there may be issues that make it difficult to change the primitive.
On Wed, May 19, 2021 at 4:05 PM Florin Mateoc florin.mateoc@gmail.com wrote:
Hi all,
In the method Process>>resume, which invokes primitive 87, the comment states that the primitive should fail if the process is already waiting in a queue. But the implementation does not do that. If we execute (inspect) the snippet:
| evaluationProcess result delay testProcess | delay := Delay forMilliseconds: 50. testProcess := Processor activeProcess. evaluationProcess := [ delay unschedule. result := testProcess isBlocked. testProcess resume] fork. delay wait. result
it returns true (and no primitive failed error occurs), which means we successfully resumed a blocked (waiting on a semaphore) process. I also checked in VMMaker and indeed, the code does not check if the process is on a queue or not before just putting it on a runnable list.
What do you think? Should the comment be changed or the implementation?
Florin
-- _,,,^..^,,,_ best, Eliot
Hi Eliot,
Indeed, that's where I discovered the anomaly - but I think that is not a good justification, that method looks bogus to me. It does not actually need to resume anything, it should read
evaluationProcess := [ result := aBlock value. evaluated := true. delay signalWaitingProcess; unschedule. ] forkNamed: 'Process to evaluate should: notTakeMoreThanMilliseconds:'.
instead of
evaluationProcess := [ result := aBlock value. evaluated := true. delay unschedule. testProcess resume ] forkNamed: 'Process to evaluate should: notTakeMoreThanMilliseconds:'.
Cheers, Florin
On Thu, May 20, 2021 at 6:15 PM Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Florin,
here's my comment in the implementation from 2010:
"Personally I would like to check MyList, which should not be one of the elements of the scheduler lists. But there are awful race conditions in things like should:notTakeMoreThan: that mean we can't. eem 9/27/2010 23:08. e.g.
So it appears that there may be issues that make it difficult to change the primitive.
On Wed, May 19, 2021 at 4:05 PM Florin Mateoc florin.mateoc@gmail.com wrote:
Hi all,
In the method Process>>resume, which invokes primitive 87, the comment states that the primitive should fail if the process is already waiting in a queue. But the implementation does not do that. If we execute (inspect) the snippet:
| evaluationProcess result delay testProcess | delay := Delay forMilliseconds: 50. testProcess := Processor activeProcess. evaluationProcess := [ delay unschedule. result := testProcess isBlocked. testProcess resume] fork. delay wait. result
it returns true (and no primitive failed error occurs), which means we successfully resumed a blocked (waiting on a semaphore) process. I also checked in VMMaker and indeed, the code does not check if the process is on a queue or not before just putting it on a runnable list.
What do you think? Should the comment be changed or the implementation?
Florin
-- _,,,^..^,,,_ best, Eliot
vm-dev@lists.squeakfoundation.org