Bug whith contextOn:do:

> On Wed, Apr 1, 2015 at 6:27 AM, Bert Freudenberg <[hidden email]> wrote:
>  
> On 31.03.2015, at 23:10, Eliot Miranda <[hidden email]> wrote:
>>
>> I'm not sure the other ways are any better.  The way to transfer to a context without disturbing its stack is to do a process switch.
>> [...]
>>   One of the advantages the process switch versions have is in not updating the receiving context sp there's a chance the context-to-stack mapping machinery won't flush the context to the heap.  In the end it might actually be faster.
>
> I would find it very surprising if #jump would result in the execution continuing in a different process.
>
> That's not what they do.  Instead they spawn another process to position the process in which we want to jump correctly.

> Hi Tobias,
>
>     first let me sympathize.  This is such horrible code :-(.  Ovr the years, getting this code to work with Cog's context-to-stack-mapping machinery has given me headaches :-).
>
>
> On Tue, Mar 31, 2015 at 9:01 AM, Tobias Pape <[hidden email]> wrote:
> Hey fellow Squeakers
>
> [Attention: low level lengthy stuff]
>
> I today encountered a strange behavior.
> When running the Startup code, somewhen ContextPart>>#complete is called,
> which, in the process issues #contextOn:do:, which subsequently somewhere interanlly
> does a jump:
>
> contextOn: exceptionClass do: block
>         "Create an #on:do: context that is ready to return from executing its receiver"
>
>         | ctxt chain |
>         ctxt := thisContext.
>         [chain := thisContext sender cut: ctxt. ctxt jump] on: exceptionClass do: block.
>         "jump above will resume here without unwinding chain"
>         ^ chain
>
> The idea is that we end up right before '^ chain' as the comment indicates.
> so what happens is that ctxt is the context of #contextOn:do: itself and it gets
> send #jump in the closure.
>
> Jump now does the following:
>
> jump
>         "Abandon thisContext and resume self instead (using the same current process).  You may want to save thisContext's sender before calling this so you can jump back to it.
>         Self MUST BE a top context (ie. a suspended context or a abandoned context that was jumped out of).  A top context already has its return value on its stack (see Interpreter>>primitiveSuspend and other suspending primitives).
>         thisContext's sender is converted to a top context (by pushing a nil return value on its stack) so it can be jump back to."
>
>         | top |
>         "Make abandoned context a top context (has return value (nil)) so it can be jumped back to"
>         thisContext sender push: nil.
>
>         "Pop self return value then return it to self (since we jump to self by returning to it)"
>         stackp = 0 ifTrue: [self stepToSendOrReturn].
>         stackp = 0 ifTrue: [self push: nil].  "must be quick return self/constant"
>         top := self pop.
>         thisContext privSender: self.
>         ^ top
>
> So. bytecode for #contextOn:do: is:
>
> 29 <8A 01> push: (Array new: 1)
> 31 <6B> popIntoTemp: 3
> 32 <89> pushThisContext:
> 33 <6A> popIntoTemp: 2
> 34 <12> pushTemp: 2
> 35 <13> pushTemp: 3
> 36 <8F 20 00 0A> closureNumCopied: 2 numArgs: 0 bytes 40 to 49
> 40      <89> pushThisContext:
> 41      <D2> send: sender
> 42      <10> pushTemp: 0
> 43      <E1> send: cut:
> 44      <8E 00 01> popIntoTemp: 0 inVectorAt: 1
> 47      <10> pushTemp: 0
> 48      <D3> send: jump
> 49      <7D> blockReturn
> 50 <10> pushTemp: 0
> 51 <11> pushTemp: 1
> 52 <F0> send: on:do:
> 53 <87> pop
> 54 <8C 00 03> pushTemp: 0 inVectorAt: 3
> 57 <7C> returnTop
>
>
> The jump lands right at 53 and does a pop.
> HOWEVER, at this point the stack of this context is empty and the pop actually pops the 3rd temp
> from the temps that 'just happens' to be right under the stack. This should be fatal.
> HOWEVER again, Squeak actually does not pop but only decrement the SP so the temp access still
> works(this _could_ be fine but some  implementations (Eg, RSqueak) tried to separate temps and
> stack; which is not possible currently).
>
> What could be the problem here?
> - are the 'stackp = 0'-checks in #jump wrong and they actually should check for the actual stack depth _after_ temps?
>
> It does look like it.  I would have expected this to be more correct:
>
> jump
> "Abandon thisContext and resume self instead (using the same current process).
> You may want to save thisContext's sender before calling this so you can jump back to it.
> Self MUST BE a top context (ie. a suspended context or a abandoned context that was jumped
> out of).  A top context already has its return value on its stack (see Interpreter>>primitiveSuspend
> and other suspending primitives). thisContext's sender is converted to a top context (by pushing a
> nil return value on its stack) so it can be jump back to."
>
> | top |
> "Make abandoned context a top context (has return value (nil)) so it can be jumped back to"
> thisContext sender push: nil.
>
> "Pop self return value then return it to self (since we jump to self by returning to it)"
> stackp <= self numTemps ifTrue: [self stepToSendOrReturn].
> (stackp <= self numTemps
> and: [self willJustPop]) ifTrue: [self push: nil].  "must be quick return self/constant"
>
> top := self pop.
> thisContext privSender: self.
> ^top
>  
> - should we put in a "sacrificial anode" in #contextOn:do: so that the pop does not pop the empty stack? (like this:
>
> contextOn: exceptionClass do: block
>         "Create an #on:do: context that is ready to return from executing its receiver"
>
>         | ctxt chain |
>         ctxt := thisContext.
>         [chain := thisContext sender cut: ctxt.
>          ctxt push: nil. "sacrifical anode"
>          ctxt jump
>         ] on: exceptionClass do: block.
>         "jump above will resume here without unwinding chain"
>         ^ chain
>
> That looks right.  Once the on:do: is sent the thisContext of contextOn:do:'s stack contains only exceptionClass, block, context and the indirection vector containing chain.  So it is in the stack = self numTemps case.
>  
>
> - Or is there an even better way?
>
> I'm not sure the other ways are any better.  The way to transfer to a context without disturbing its stack is to do a process switch.  So you do something equivalent to
>
> jump
> "Abandon thisContext and resume self instead (using the same current process)."
>
> | process semaphore |
> process := Processor activeProcess.
> semaphore := Semaphore new.
>
> [process suspendedContext unwindTo: self.
> process suspendedContext: self.
> semaphore signal] fork.
>
> semaphore wait
>
> This way no bizarre stack manipulations are going on, and no return value is pushed, because there isn't a return.  One may get away with:
>
> jump
> "Abandon thisContext and resume self instead (using the same current process)."
>
> [| process |
> process := Processor activeProcess.
> process suspendedContext unwindTo: self.
> process suspendedContext: self] fork.
>
> Processor yield
>
> I'd be interested in your experience using either of these.  One of the advantages the process switch versions have is in not updating the receiving context sp there's a chance the context-to-stack mapping machinery won't flush the context to the heap.  In the end it might actually be faster.
> --
> best,
> Eliot