On Mon, Dec 5, 2011 at 9:13 AM, Stéphane Ducasse <stephane.ducasse@inria.fr>wrote:
On Dec 5, 2011, at 2:58 PM, Marcus Denker wrote:
Hi,
With this change set, we get an error when doing the following:
In the workspace do a "debug it" on this code:
Transcript show: Class name
then two times "step over" --> Subscript out of bounds error. Without the change it works.
this shows that squeak related fix should be applied with care and that we should understand deeply our system.
Not really. That was not a fix. It was a potential fix that I was asking people to test. Marcus found a test case that demonstrated a problem with my fix. With Marcus' case I was able to correct my fix and today Levente committed my revised fix to Squeak and Marcus has updated Pharo. So things worked well. I found a bug, developed a partial fix, asked for testing help, got it, revised my fix. And this in a very difficult to understand part of the system. I'm trying but I'm not happy with my knowledge on certain parts. But since
there is little documentation and some abstractions are not aligned, this makes everything more challenging.
When I see the senders, I think that it will be delicate to fix
Process>>terminate "Stop the process that the receiver represents forever. Unwind to execute pending ensure:/ifCurtailed: blocks before terminating."
| ctxt unwindBlock oldList | self isActiveProcess ifTrue: [ ctxt := thisContext. [ ctxt := ctxt findNextUnwindContextUpTo: nil. ctxt isNil ] whileFalse: [ (ctxt tempAt: 2) ifNil: [ ctxt tempAt: 2 put: nil. unwindBlock := ctxt tempAt: 1. thisContext terminateTo: ctxt. unwindBlock value ]]. thisContext terminateTo: nil. self suspend ] ifFalse: [ "Always suspend the process first so it doesn't accidentally get woken up" oldList := self suspend. suspendedContext ifNotNil:[ "Figure out if we are terminating the process while waiting in Semaphore>>critical: In this case, pop the suspendedContext so that we leave the ensure: block inside Semaphore>>critical: without signaling the semaphore." (oldList class == Semaphore and:[ suspendedContext method == (Semaphore compiledMethodAt: #critical:)]) ifTrue:[ suspendedContext := suspendedContext home.].
"If we are terminating a process halfways through an unwind, try to complete that unwind block first." (suspendedContext findNextUnwindContextUpTo: nil) ifNotNil: [ :outer | (suspendedContext findContextSuchThat: [ :c | c closure == (outer tempAt: 1)]) ifNotNil: [ :inner | "This is an unwind block currently under evaluation" suspendedContext runUntilErrorOrReturnFrom: inner ]]. ctxt := self popTo: suspendedContext bottomContext. ctxt == suspendedContext bottomContext ifFalse: [ self debug: ctxt title: 'Unwind error during termination']] ].
Process>>complete: aContext "Run self until aContext is popped or an unhandled error is raised. Return self's new top context, unless an unhandled error was raised then return the signaler context (rather than open a debugger)."
| ctxt pair error | ctxt := suspendedContext. suspendedContext := nil. "disable this process while running its stack in active process below" pair := ctxt runUntilErrorOrReturnFrom: aContext. suspendedContext := pair first. error := pair second. error ifNotNil: [^ error signalerContext]. ^ suspendedContext
On Wed, Nov 16, 2011 at 8:58 PM, Eliot Miranda <eliot.miranda@gmail.com>
wrote:
Hi All,
I've found a bug in ContextPart>>#runUntilErrorOrReturnFrom: that in a certain rare circumstance causes the method to peel back too much of the stack. This method is used in the debugger (e.g. completeStep: et al) but also in Process termination. So it is right at the heart of the system. I'm loath to simply commit it I'd really appreciate y'all testing with the fix and reporting any changes you see. It does not affect the trunk test suite (there is a test, WeakSetInspectorTest, which is unreliable, but apart form that no observed effects). So, brave souls, please try with the attached and see if you come to grief or not. -- thanks, Eliot
-- -- Marcus Denker -- denker@acm.org http://www.marcusdenker.de
-- best, Eliot