--==CelesteAttachment21924== Content-type: text/plain;charset=UTF-8
Hi Eliot, all,
There's a bug, or rather an omission, in #releaseCriticalSection: causing the following examples unwind incorrectly:
Mutex new inspect critical: [self halt]
If you run the example in a workspace, a debugger and an inspector opens - please watch Mutex's 'owner' variable; then step into a few times to get right before sending #primitiveExitCriticalSection and terminate the debugger (in the newest images please use debugger's window menu -> terminate process); and observe the owner won't get cleared and will remain blocking the Mutex. Same behavior for Semaphores:
Semaphore forMutualExclusion inspect critical: [self halt]
Again, watch Semaphore's excessSignals variable in the Inspector, step into a few times until you stand before sending #signal - and terminate the debugged process (from the debuggers menu or from the Process Browser if you like) - the Semaphore won't recharge excessSignals back to 1.
The root cause is #releaseCriticalSection: doesn't account for terminating in this particular position of the computation. It can be fixed e.g. by adding an additional condition ('progressedIntoEnsure') - try the examples with the enclosed changeset; both Mutexes and Semaphores work ok now.
Best,
~~~ ^[^ Jaromir
Sent from Squeak Inbox Talk ["Process-releaseCriticalSection.st"] --==CelesteAttachment21924== Content-transfer-encoding: base64 Content-disposition: attachment;filename="Process-releaseCriticalSection.st" Content-type: application/octet-stream;name="Process-releaseCriticalSection.st"
J0Zyb20gU3F1ZWFrNi4wYWxwaGEgb2YgMTcgRGVjZW1iZXIgMjAyMSBbbGF0ZXN0IHVwZGF0 ZTogIzIwODgwXSBvbiAyMSBEZWNlbWJlciAyMDIxIGF0IDY6MDQgcG0nIQ0NIVByb2Nlc3Mg bWV0aG9kc0ZvcjogJ3ByaXZhdGUnIHN0YW1wOiAnamFyIDEyLzIxLzIwMjEgMTY6MzUnIQ1y ZWxlYXNlQ3JpdGljYWxTZWN0aW9uOiBydW5uYWJsZQ0JIkZpZ3VyZSBvdXQgaWYgd2UgYXJl IHRlcm1pbmF0aW5nIGEgcHJvY2VzcyB0aGF0IGlzIGluIHRoZSBlbnN1cmU6IGJsb2NrIG9m IGEgY3JpdGljYWwgc2VjdGlvbi4NCSBJbiB0aGlzIGNhc2UsIGlmIHRoZSBibG9jayBoYXMg bWFkZSBwcm9ncmVzcywgcG9wIHRoZSBzdXNwZW5kZWRDb250ZXh0IHNvIHRoYXQgd2UgbGVh dmUgdGhlDQkgZW5zdXJlOiBibG9jayBpbnNpZGUgdGhlIGNyaXRpY2FsOiB3aXRob3V0IHNp Z25hbGluZyB0aGUgc2VtYXBob3JlL2V4aXRpbmcgdGhlIHByaW1pdGl2ZSBzZWN0aW9uLA0J IHNpbmNlIHByZXN1bWFibHkgdGhpcyBoYXMgYWxyZWFkeSBoYXBwZW5lZC4gIEJ1dCBpZiBp dCBoYXNuJ3QgbWFkZSBwcm9ncmVzcyBidXQgaXMgYmV5b25kIHRoZQ0JIHdhaXQgKHdoaWNo IHdlIGNhbiB0ZWxsIGJ5IHRoZSBvbGRMaXN0IGJlaW5nIG9uZSBvZiB0aGUgcnVubmFibGUg bGlzdHMsIGkuZS4gYSBMaW5rZWRMaXN0LCBub3QgYQ0JIFNlbWFwaG9yZSBvciBNdXRleCwg ZXQgYWwpLCB0aGVuIHRoZSBlbnN1cmU6IGJsb2NrIG5lZWRzIHRvIGJlIHJ1bi4iDQl8IHNl bGVjdG9ySnVzdFNlbnQgfA0JKHN1c3BlbmRlZENvbnRleHQgbWV0aG9kIHByYWdtYUF0OiAj Y3JpdGljYWxTZWN0aW9uKSBpZk5pbDogW15zZWxmXS4NCXNlbGVjdG9ySnVzdFNlbnQgOj0g c3VzcGVuZGVkQ29udGV4dCBzZWxlY3Rvckp1c3RTZW50T3JTZWxmLg0NCSJSZWNlaXZlciBh bmQvb3IgYXJndW1lbnQgYmxvY2tzIG9mIGVuc3VyZTogaW4gU2VtYXBob3JlPj5jcml0aWNh bDogb3IgTXV0ZXg+PiNjcml0aWNhbDoiDQlzdXNwZW5kZWRDb250ZXh0IGlzQ2xvc3VyZUNv bnRleHQgaWZUcnVlOg0JCVtzdXNwZW5kZWRDb250ZXh0IHNlbmRlciBpc1Vud2luZENvbnRl eHQgaWZUcnVlOg0JCQlbfCBub3RXYWl0aW5nQnV0TWFkZU5vUHJvZ3Jlc3MgcHJvZ3Jlc3Nl ZEludG9FbnN1cmUgfA0JCQkiQXZvaWQgcnVubmluZyB0aGUgZW5zdXJlOiBibG9jayB0d2lj ZSwgcG9wcGluZyBpdCBpZiBpdCBoYXMgYWxyZWFkeSBiZWVuIHJ1bi4gSWYgcnVubmFibGUN CQkJIGJ1dCBhdCB0aGUgd2FpdCwgbGVhdmUgaXQgaW4gcGxhY2UuIE4uQi4gTm8gbmVlZCB0 byBjaGVjayBpZiB0aGUgYmxvY2sgcmVjZWl2ZXIgb2YgZW5zdXJlOiBoYXMNCQkJIG5vdCBz dGFydGVkIHRvIHJ1biAodmlhIHN1c3BlbmRlZENvbnRleHQgcGMgPSBzdXNwZW5kZWRDb250 ZXh0IHN0YXJ0cGMpIGJlY2F1c2UgZW5zdXJlOg0JCQkgdXNlcyB2YWx1ZU5vQ29udGV4dFN3 aXRjaCwgYW5kIHNvIHRoZXJlIGlzIG5vIHN1c3BlbnNpb24gcG9pbnQgYmVmb3JlIHRoZSB3 YWl0LiINCQkJIG5vdFdhaXRpbmdCdXRNYWRlTm9Qcm9ncmVzcyA6PQ0JCQkJcnVubmFibGUN CQkJCWFuZDogW3NlbGVjdG9ySnVzdFNlbnQgPT0gI3dhaXQNCQkJCWFuZDogW3N1c3BlbmRl ZENvbnRleHQgc2VuZGVyIHNlbGVjdG9ySnVzdFNlbnRPclNlbGYgPT0gI3ZhbHVlTm9Db250 ZXh0U3dpdGNoXV0uDQkJCSBwcm9ncmVzc2VkSW50b0Vuc3VyZSA6PQ0JCQkJcnVubmFibGUN CQkJCWFuZDogW3N1c3BlbmRlZENvbnRleHQgc2VuZGVyIHNlbGVjdG9ySnVzdFNlbnRPclNl bGYgPT0gI3ZhbHVlXS4NCQkJIChub3RXYWl0aW5nQnV0TWFkZU5vUHJvZ3Jlc3Mgb3I6IFtw cm9ncmVzc2VkSW50b0Vuc3VyZV0pIGlmRmFsc2U6DQkJCQlbc3VzcGVuZGVkQ29udGV4dCA6 PSBzdXNwZW5kZWRDb250ZXh0IGhvbWVdXS4NCQkgXnNlbGZdLg0NCSJFaXRoZXIgU2VtYXBo b3JlPj5jcml0aWNhbDogb3IgTXV0ZXg+PiNjcml0aWNhbDouICBJcyB0aGUgcHJvY2VzcyBz dGlsbCBibG9ja2VkPyAgSWYgc28sIG5vdGhpbmcgZnVydGhlciB0byBkby4iDQlydW5uYWJs ZSBpZkZhbHNlOiBbXnNlbGZdLg0NCSJJZiBzdGlsbCBhdCB0aGUgd2FpdCB0aGUgZW5zdXJl OiBibG9jayBoYXMgbm90IGJlZW4gYWN0aXZhdGVkLCBzbyBzaWduYWwgdG8gcmVzdG9yZS4i DQlzZWxlY3Rvckp1c3RTZW50ID09ICN3YWl0IGlmVHJ1ZToNCQlbc3VzcGVuZGVkQ29udGV4 dCByZWNlaXZlciBzaWduYWxdLg0NCSJJZiBzdGlsbCBhdCB0aGUgbG9jayBwcmltaXRpdmUg YW5kIHRoZSBsb2NrIHByaW1pdGl2ZSBqdXN0IGFjcXVpcmVkIG93bmVyc2hpcCAoaW5kaWNh dGVkIGJ5IGl0IGFuc3dlcmluZyBmYWxzZSkNCSB0aGVuIHRoZSBlbnN1cmUgYmxvY2sgaGFz IG5vdCBiZWVuIGFjdGl2YXRlZCwgc28gZXhwbGljaXRseSBwcmltaXRpdmVFeGl0Q3JpdGlj YWxTZWN0aW9uIHRvIHVubG9jay4iDQkoc2VsZWN0b3JKdXN0U2VudCA9PSAjcHJpbWl0aXZl RW50ZXJDcml0aWNhbFNlY3Rpb24NCSBvcjogW3NlbGVjdG9ySnVzdFNlbnQgPT0gI3ByaW1p dGl2ZVRlc3RBbmRTZXRPd25lcnNoaXBPZkNyaXRpY2FsU2VjdGlvbl0pIGlmVHJ1ZToNCQlb KHN1c3BlbmRlZENvbnRleHQgc3RhY2tQdHIgPiAwDQkJICBhbmQ6IFtzdXNwZW5kZWRDb250 ZXh0IHRvcCA9PSBmYWxzZV0pIGlmVHJ1ZToNCQkJW3N1c3BlbmRlZENvbnRleHQgcmVjZWl2 ZXIgcHJpbWl0aXZlRXhpdENyaXRpY2FsU2VjdGlvbl1dISAhDQ== --==CelesteAttachment21924==-- --=--
Hi Jaromir --
What is the current status of this? :-)
Best, Marcel Am 21.12.2021 18:14:21 schrieb mail@jaromir.net mail@jaromir.net: Hi Eliot, all,
There's a bug, or rather an omission, in #releaseCriticalSection: causing the following examples unwind incorrectly:
Mutex new inspect critical: [self halt]
If you run the example in a workspace, a debugger and an inspector opens - please watch Mutex's 'owner' variable; then step into a few times to get right before sending #primitiveExitCriticalSection and terminate the debugger (in the newest images please use debugger's window menu -> terminate process); and observe the owner won't get cleared and will remain blocking the Mutex. Same behavior for Semaphores:
Semaphore forMutualExclusion inspect critical: [self halt]
Again, watch Semaphore's excessSignals variable in the Inspector, step into a few times until you stand before sending #signal - and terminate the debugged process (from the debuggers menu or from the Process Browser if you like) - the Semaphore won't recharge excessSignals back to 1.
The root cause is #releaseCriticalSection: doesn't account for terminating in this particular position of the computation. It can be fixed e.g. by adding an additional condition ('progressedIntoEnsure') - try the examples with the enclosed changeset; both Mutexes and Semaphores work ok now.
Best,
~~~ ^[^ Jaromir
Sent from Squeak Inbox Talk ["Process-releaseCriticalSection.st"]
Hi Marcel,
The issue is fixed for #terminate by using #suspendAndReleaseCriticalSection and the corresponding tests are #testMutexInCriticalEnsureArgument and #testSemaInCriticalEnsureArgument; however, the issue still remains in #releaseCriticalSection: which is being used by #terminateAggressively which means if you run the same example but Abandon the debugger instead of terminating it the Mutex's owner won't get cleared which is the bug.
To summarize the root cause: the code in #releaseCriticalSection: tries to deal with three possible implementations of #critical (see https://forum.world.st/Solving-termination-of-critical-sections-in-the-conte...); eventually version V1 was adopted but the code dealing with V2 remained and left a hole causing V1 fail in the case showed by the example (below).
The fix was that I used only the relevant code from #releaseCriticalSection: in #suspendAndReleaseCriticalSection being used by #terminate.
The question remains what to do with #terminateAggressively (Abandon); I've just tried to simply replace #releaseCriticalSection: with #suspendAndReleaseCriticalSection but that doesn't seem to help and my original fix doesn't work for #terminateAggressively either which means there may be something else at play here.
Thanks, Jaromir
From: Marcel Taeumelmailto:marcel.taeumel@hpi.de Sent: Thursday, July 7, 2022 15:00 To: squeak-devmailto:squeak-dev@lists.squeakfoundation.org Subject: Re: [squeak-dev] A bug in #releaseCriticalSection: preventing correct unwind (in rare situations)
Hi Jaromir --
What is the current status of this? :-)
Best, Marcel
Am 21.12.2021 18:14:21 schrieb mail@jaromir.net mail@jaromir.net: Hi Eliot, all,
There's a bug, or rather an omission, in #releaseCriticalSection: causing the following examples unwind incorrectly:
Mutex new inspect critical: [self halt]
If you run the example in a workspace, a debugger and an inspector opens - please watch Mutex's 'owner' variable; then step into a few times to get right before sending #primitiveExitCriticalSection and terminate the debugger (in the newest images please use debugger's window menu -> terminate process); and observe the owner won't get cleared and will remain blocking the Mutex. Same behavior for Semaphores:
Semaphore forMutualExclusion inspect critical: [self halt]
Again, watch Semaphore's excessSignals variable in the Inspector, step into a few times until you stand before sending #signal - and terminate the debugged process (from the debuggers menu or from the Process Browser if you like) - the Semaphore won't recharge excessSignals back to 1.
The root cause is #releaseCriticalSection: doesn't account for terminating in this particular position of the computation. It can be fixed e.g. by adding an additional condition ('progressedIntoEnsure') - try the examples with the enclosed changeset; both Mutexes and Semaphores work ok now.
Best,
~~~ ^[^ Jaromir
Sent from Squeak Inbox Talk ["Process-releaseCriticalSection.st"]
squeak-dev@lists.squeakfoundation.org