[squeak-dev] Two more bugs: #terminate: allows (1) multiple termination and (2) resuming a process being terminated

Marcel Taeumel marcel.taeumel at hpi.de
Fri Dec 17 13:46:36 UTC 2021


Hi Jaromir --

I really appreciate the amount of commentary you add around such a critical piece of source code. ^__^ Thanks!

Best,
Marcel
Am 17.12.2021 13:32:15 schrieb mail at jaromir.net <mail at jaromir.net>:
Hi all,
A proposal to prevent multiple termination and/or resuming of a terminating process is in Kernel-jar.1436 and the corresponding tests in KernelTests-jar.417

Please review if the proposed solution is acceptable (setting suspendedContext to nil during the whole termination, including during execution of #releaseCriticalSection). For the sake of simplicity I haven't changed #releaseCriticalSection code except shadowing the suspendedContext instance variable by a local variable. It can be of course changed but didn't want to overcomplicate things more than they are :)

Thanks for your time,
~~~
^[^ Jaromir

Sent from Squeak Inbox Talk

On 2021-12-17T12:13:53+01:00, mail at jaromir.net wrote:

> This issue exists independently of the new #teminate in Kernel-jar.1435. The problem is:
>
> If a process terminating another process gets suspended in the middle, presently there is no mechanism that would prevent a terminating process to be resumed or terminated again. When the process executing the initial termination resumes eventually the state of its computation may be troubling (suspendedContext or pc may be nil etc.) resulting in a nonsensical continuation until some sort of end which may be an error or potentially a crash. In addition, resuming a process that has already started its termination doesn't make much sense either.
>
> Below I'm enclosing two simple tests illustrating these two scenarios.
>
> One possible mechanism to prevent both scenarios is to set suspendedContext to nil during the whole termination (including during execution of #releaseCriticalSection); there are some considerations though:
> (1) #releaseCriticalSection uses the instance variable suspendedContext which would be replaced by its copy in a local variable and #releaseCriticalSection would move under Context
> (2) the teminating process would be answering 'false' to isTerminated during its termination, i.e. sort of lying ;) Would it be worth the effort to fix it somehow? Or can we just leave it? We might need a new state to do this...
> (3) accidental resume sent to a terminating process would result in an error (which may even be desirable)
>
> In case the above points are not acceptable, a more robust solution might introduce an additional boolean state to Process indicating a process is being terminated (but I'd prefer to make this work first without introducing any new state).
>
> The following two tests fail (from do-it) with the current and previous trunk versions of #terminate.
>
> The first shows that when termination gets suspended for any reason (higher priority preemption etc) then the original process may accidentaly get resumed which is definitely not making good sense.
>
> The second shows similarly the original process may accidentaly get terminated again which leaves the original termination in a nonsensical state leading to an error.
>
> "testResumeTerminatingProcess"
>
> | terminatee terminator resumed semaphore |
> semaphore := Semaphore new.
> terminatee := [semaphore critical:[]. resumed := true] fork.
> Processor yield.
> terminator := [terminatee terminate] newProcess.
> self assert: terminatee suspendingList == semaphore.
> self assert: terminator isSuspended.
> "run terminator and stop inside #terminate"
> terminator runUntil: [:ctx | ctx selectorToSendOrSelf = #findNextUnwindContextUpTo:].
> self assert: terminator isSuspended.
> "resume the terminatee process and let it finish normally"
> terminatee resume.
> Processor yield.
> self assert: resumed isNil.
> "now let the terminator continue terminating the terminatee process"
> terminator resume.
> Processor yield.
> self assert: terminator isTerminated.
> self logExit
>
> "testTerminateTerminatingProcess"
>
> | terminatee terminator resumed semaphore |
> semaphore := Semaphore new.
> terminatee := [semaphore critical:[]. resumed := true] fork.
> Processor yield.
> terminator := [terminatee terminate] newProcess.
> self assert: terminatee suspendingList == semaphore.
> self assert: terminator isSuspended.
> "run terminator and stop inside #releaseCriticalSection:"
> terminator runUntil: [:ctx | ctx selectorToSendOrSelf = #selectorJustSentOrSelf].
> self assert: terminator isSuspended.
> "terminate the terminatee process again and let the termination finish"
> terminatee terminate.
> self assert: resumed isNil.
> self assert: terminatee isTerminated.
> "now let the terminator continue terminating the terminatee process"
> terminator resume.
> Processor yield.
> self assert: terminator isTerminated.
> self logExit
>
>
> I'll send a proposed fix in the Inbox for your consideration.
>
> Thanks,
> Best,
>
> ~~~
> ^[^ Jaromir
>
> Sent from Squeak Inbox Talk
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20211217/89baa327/attachment.html>


More information about the Squeak-dev mailing list