#ensure: issues (was: Re: [Pharo-project] Pharo by Example vol 2: new chapter available)

What do you guys think about this? It behaves no worse than the current implementation if it encountered a problematic ensure: block in #terminate but it does try to complete one when it finds it. I think that short of changing the overall terminate semantics this is pretty close to as good as it gets. The test illustrates the problem.

Eliot - could you check that my usage of findContextSuchThat / closures / runUntilErrorOrReturn: looks reasonable? I'm still relatively new to closure land in these areas.

Cheers,
 - Andreas

Certainly a tangled web... of course we want all ensures to happen,
like, I think, any use of terminate to happen too. A conflict of
interest, especially in the case of errors within ensures...

No easy answer I think. I favour having terminate complete if errors in
ensures, otherwise that ensures complete before eventual termination...

Regards, Gary

On Thu, 2010-03-04 at 00:56 +0100, Nicolas Cellier wrote:
> 2010/3/4 Levente Uzonyi <[hidden email]>:
> > On Thu, 4 Mar 2010, Nicolas Cellier wrote:
> >
> >> 2010/3/3 Levente Uzonyi <[hidden email]>:
> >>>
> >>> On Wed, 3 Mar 2010, Andreas Raab wrote:
> >>>
> >>>> On 3/3/2010 2:07 PM, Levente Uzonyi wrote:
> >>>>>
> >>>>> On Wed, 3 Mar 2010, Igor Stasenko wrote:
> >>>>>>
> >>>>>> i don't get it. Just before that, you said: ' I'd expect it to be
> >>>>>> evaluated no matter what happens.' ?
> >>>>>> But now you saying that it may not be executed in some conditions
> >>>>>> (when user pressing abandon button, causing process to be terminated).
> >>>>>
> >>>>> It's simple: don't terminate process X from another process if process
> >>>>> X
> >>>>> is executing a termiation block (aka #ensure: block). Or if you
> >>>>> terminate it, make sure that the execution of the block will continue
> >>>>> somehow (I don't care how).
> >>>>
> >>>> You're missing Igors point which is that in his example the halt /
> >>>> Transcript *was* in the ensure block and as a result you're
> >>>> contradicting
> >>>> yourself here. Let's go back to Igor's example:
> >>>>
> >>>> [self boom ] ensure: [ self halt. Transcript show: 'boom']
> >>>>
> >>>> The halt is inside the ensure block. If you terminate the process from
> >>>> the
> >>>> debugger, it would be logical from your statement that the Transcript
> >>>> message would be executed - after all it's " executing a termiation
> >>>> block
> >>>> (aka #ensure: block)" and so it can't be terminated by your reasoning.
> >>>> However, when Igor was pointing this out you replied with "I didn't say
> >>>> that. I said evaluate it the same way as normal code." which is
> >>>> inconsistent
> >>>> with the other statement.
> >>>
> >>> That shows my lack of knowledge about how the debugger works.
> >>>
> >>>>
> >>>>> I think every user of #ensure: expects that the termination blocks are
> >>>>> executed even if the process which is executing the receiver of
> >>>>> #ensure:
> >>>>> is terminated. And it actually happens in all but this case.
> >>>>
> >>>> The question of terminating processes is always tricky. I don't think
> >>>> that
> >>>> your proposal would actually work in practice - it could easily result
> >>>> in
> >>>> processes that cannot be terminated due to a simple bug in an ensure
> >>>> block.
> >>>> Personally, I'd rather say that the more useful behavior would be
> >>>> something
> >>>> along the lines of saying that process termination either skips the
> >>>> current
> >>>> ensure block (assuming there's a bug and it should get the heck out of
> >>>> it
> >>>> but try to evaluate the remaining ones) or that there need to be two
> >>>> terminations - one that is 'soft' and won't allow ensure blocks to be
> >>>> skipped and one that is 'hard' (kill -9 hard) and just ignores all the
> >>>> ensure blocks.
> >>>
> >>> I'm only saying that normal usage (aka #terminate) shouldn't do
> >>> unexpected
> >>> things like this.
> >>> If you read the comment of Process >> #terminate, you may assume that
> >>> #ensure: and #ifCurtailed: blocks will be excuted even if you use
> >>> #terminate, but that's not true.
> >>>
> >>> "Stop the process that the receiver represents forever.  Unwind to
> >>> execute
> >>> pending ensure:/ifCurtailed: blocks before terminating."
> >>>
> >>>
> >>> Levente
> >>>
> >>
> >> The only way I see to solve your problem would be to execute the
> >> unwind block in another process...
> >> Quite technical and costly !
> >
> > It's our problem. Just look at the senders of #ensure: and imagine what will
> > happen if the termination block is not evaluated.
> > I think there's another way (though it might be my lack of knowledge again).
> > After suspending the process which is about to be terminated we can check if
> > it's executing a termination block. It it's not, we are safe to continue the
> > termination, otherwise we can do something else which ensures that the
> > termination block is evaluated.
> >
>
> Maybe...
> Unfortunately, you did not tell how you will distinguish well behaved
> unwind-blocks from Igor's example...
>
> Nicolas
>
> >
> > Levente
> >
> >>
> >> Nicolas
> >>
> >>>>
> >>>> Cheers,
> >>>>  - Andreas
> >>>>
> >>>>
> >>>
> >>>
> >>
> >
> >
> >
> >
>

On 3/3/2010 5:23 PM, Eliot Miranda wrote:

On Wed, Mar 3, 2010 at 5:07 PM, Andreas Raab <[hidden email]

<mailto:[hidden email]>> wrote:

   What do you guys think about this? It behaves no worse than the
   current implementation if it encountered a problematic ensure: block
   in #terminate but it does try to complete one when it finds it. I
   think that short of changing the overall terminate semantics this is
   pretty close to as good as it gets. The test illustrates the problem.

   Eliot - could you check that my usage of findContextSuchThat /
   closures / runUntilErrorOrReturn: looks reasonable? I'm still
   relatively new to closure land in these areas.


That's fine.  But running unwinds in a different process is, I think,
unwise.  Process identity is important and unwinds should be run in the
context of the process itself.

I said "short of changing the overall termination semantics" :-)

Cheers,
 - Andreas

 Here's the VisualWorks code:

terminate
"Terminate the receiver process, by sending the Process terminateSignal.
Allow all unwind blocks to run, even if they are currently in progress."

"If the receiver is the active process then raise the terminateSignal
 which should be
caught by the handler in Process class>>forBlock:priority:.  The handler
will return,
causing all unwind blocks to run, and then will invoke finalTerminate,
actually terminating the receiver."
Processor activeProcess == self ifTrue:
[ [ Process terminateSignal raise ] on: Signal noHandlerSignal do:
[ :ex |
"Usually, this can only happen if a Process was terminated unnaturally.
Try to recover gracefully."
Process terminateSignal == ex unhandledException class
ifTrue: [ ex return ]
ifFalse: [ ex pass ].
].
^self finalTerminate
].

"If the receiver is not the active process and its suspendedContext is
nil then
it is already suspended."
suspendedContext == nil ifTrue: [^self].

"Suspend the receiver, then place a block atop the receiver's stack to
invoke this method as the active process, and resume the receiver."
interruptProtect critical:
[| savedContext interruptContext outerMostUnwind curr |
myList == nil ifFalse: [self suspend].
curr := suspendedContext.
[curr == nil] whileFalse:
[curr method isMarkedForUnwindInAction
ifTrue: [outerMostUnwind := curr.
curr := curr skipOverUnwindingBlocks].
curr := curr findNextMarkedUpTo: nil].
(outerMostUnwind ~~ nil and: [outerMostUnwind method numTempsOnly > 0])
ifTrue:
[outerMostUnwind localAt: outerMostUnwind method numArgs+1 put: true]
ifFalse:
[savedContext := suspendedContext.
interruptContext := [self terminate] newContext.
interruptContext sender: savedContext.
suspendedContext := interruptContext].
self resume]

So...
- it only runs unwinds in the context of the process, pushing a block
that will run terminate if other than the current process tries to
terminate it
- it tried to discover whether the attempt to terminate from another
process was made while unwind blocks are being run and marks any such
block as having completed (i.e. it'll short-circuit an already-executing
unwind block).



P.S. Here's the identification of an unwind in action:

BlockClosure>>valueAsUnwindBlockFrom: aContextOrNil
"Unwind blocks are evaluated using this wrapper.
This method is marked as special.  When the
system searches for unwind blocks, it skips over
all contexts between this context and the context
passed in as an argument.  If the argument is
nil, it skips over the sender of this context.

The purpose of this is that errors inside an
unwind block should not evaluate that unwind
block, and they should not circumvent the
running of other unwind blocks lower on the
stack."

<exception: #unwindInAction>

| shouldTerminate |
"The first temporary variable in this method is treated specially by
Process>>terminate,
which may set it to true. If that happens, terminate the process at the
end of the unwind action.
Normally we should be able to take for granted that shouldTerminate has
nil as its initial
value, but since this variable is modified by another process, it's
possible that the other
process could set the value to true when this process is waiting at the
initial PC of the method.
In that case, when this process resumes, it needs to make sure that it
doesn't overwrite
the true with a false.

On a purely theoretical level, there's still room for a race condition
between testing for nil
and assigning false to the variable, but in practise our execution
technology doesn't allow
for a process switch between the test and the assignment, so the only
way it could be a
problem in practise is if the method were being debugged or run by a VI
level interpreter
at the time that some other process wanted to terminate it. At this
time, the risk of that kind
of situation seems so much smaller thatn the risk of a process switch at
the initial PC, that
we're willing to fix only the initial PC case."
shouldTerminate == nil ifTrue: [shouldTerminate := false].
self value.
shouldTerminate
ifTrue:
[Processor activeProcess terminate].

CompiledCode>>isMarkedForUnwindInAction
"Answer true if marked for unwind in action."

^false

MarkedMethod>>isMarkedForUnwindInAction
"Answer true if method is marked for unwinding in action."

^markType == #unwindInAction
and I can spell practice ;)


HTH
Eliot


   Cheers,
     - Andreas

On 3/3/2010 5:52 PM, Eliot Miranda wrote:

But running unwinds in anything other than the owning process is
/broken/

No, Eliot, it's *different*. If it were outright broken then lots of unwind actions shouldn't work. That is not the case.

In fact, I am more and more starting to think that it may be the *right* thing to do because you get the process context associated with the terminating process allowing to do something like:

 "kill the process silently"
 [p terminate]
   on: Timeout do:[:ex| ex return "skip it"]
   on: Error do:["ignore"].

There are obviously issues if the unwind actions refer to process local state; but then again perhaps providing said context for the duration of the unwind is the right thing, along the lines of:

Process>>terminate

 self isActiveProcess ifFalse:[
   Processor activeProcess evaluate: [self unwind] onBehalfOf: self.
 ].

This for example would guard errors during unwind as initially intended while ensuring that process relative state is retrieved correctly.

In any case, I think the issue is *significantly* more faceted than just saying "it's broken".

Cheers,
 - Andreas

and arranging that a process runs its own unwinds on
termination is straight-forward.  The VW code does a few other things,
but the way it transfers control from one process to another is easy to
do in Squeak.


   Cheers,
     - Andreas


     Here's the VisualWorks code:


       terminate
       "Terminate the receiver process, by sending the Process
       terminateSignal.
       Allow all unwind blocks to run, even if they are currently in
       progress."

       "If the receiver is the active process then raise the
       terminateSignal
         which should be
       caught by the handler in Process class>>forBlock:priority:.  The
       handler
       will return,
       causing all unwind blocks to run, and then will invoke
       finalTerminate,
       actually terminating the receiver."
       Processor activeProcess == self ifTrue:
       [ [ Process terminateSignal raise ] on: Signal noHandlerSignal do:
       [ :ex |
       "Usually, this can only happen if a Process was terminated
       unnaturally.
       Try to recover gracefully."
       Process terminateSignal == ex unhandledException class
       ifTrue: [ ex return ]
       ifFalse: [ ex pass ].
       ].
       ^self finalTerminate
       ].

       "If the receiver is not the active process and its
       suspendedContext is
       nil then
       it is already suspended."
       suspendedContext == nil ifTrue: [^self].

       "Suspend the receiver, then place a block atop the receiver's
       stack to
       invoke this method as the active process, and resume the receiver."
       interruptProtect critical:
       [| savedContext interruptContext outerMostUnwind curr |
       myList == nil ifFalse: [self suspend].
       curr := suspendedContext.
       [curr == nil] whileFalse:
       [curr method isMarkedForUnwindInAction
       ifTrue: [outerMostUnwind := curr.
       curr := curr skipOverUnwindingBlocks].
       curr := curr findNextMarkedUpTo: nil].
       (outerMostUnwind ~~ nil and: [outerMostUnwind method
       numTempsOnly > 0])
       ifTrue:
       [outerMostUnwind localAt: outerMostUnwind method numArgs+1 put:
       true]
       ifFalse:
       [savedContext := suspendedContext.
       interruptContext := [self terminate] newContext.
       interruptContext sender: savedContext.
       suspendedContext := interruptContext].
       self resume]

       So...
       - it only runs unwinds in the context of the process, pushing a
       block
       that will run terminate if other than the current process tries to
       terminate it
       - it tried to discover whether the attempt to terminate from another
       process was made while unwind blocks are being run and marks any
       such
       block as having completed (i.e. it'll short-circuit an
       already-executing
       unwind block).



       P.S. Here's the identification of an unwind in action:

       BlockClosure>>valueAsUnwindBlockFrom: aContextOrNil
       "Unwind blocks are evaluated using this wrapper.
       This method is marked as special.  When the
       system searches for unwind blocks, it skips over
       all contexts between this context and the context
       passed in as an argument.  If the argument is
       nil, it skips over the sender of this context.

       The purpose of this is that errors inside an
       unwind block should not evaluate that unwind
       block, and they should not circumvent the
       running of other unwind blocks lower on the
       stack."

       <exception: #unwindInAction>

       | shouldTerminate |
       "The first temporary variable in this method is treated specially by
       Process>>terminate,
       which may set it to true. If that happens, terminate the process
       at the
       end of the unwind action.
       Normally we should be able to take for granted that
       shouldTerminate has
       nil as its initial
       value, but since this variable is modified by another process, it's
       possible that the other
       process could set the value to true when this process is waiting
       at the
       initial PC of the method.
       In that case, when this process resumes, it needs to make sure
       that it
       doesn't overwrite
       the true with a false.

       On a purely theoretical level, there's still room for a race
       condition
       between testing for nil
       and assigning false to the variable, but in practise our execution
       technology doesn't allow
       for a process switch between the test and the assignment, so the
       only
       way it could be a
       problem in practise is if the method were being debugged or run
       by a VI
       level interpreter
       at the time that some other process wanted to terminate it. At this
       time, the risk of that kind
       of situation seems so much smaller thatn the risk of a process
       switch at
       the initial PC, that
       we're willing to fix only the initial PC case."
       shouldTerminate == nil ifTrue: [shouldTerminate := false].
       self value.
       shouldTerminate
       ifTrue:
       [Processor activeProcess terminate].

       CompiledCode>>isMarkedForUnwindInAction
       "Answer true if marked for unwind in action."

       ^false

       MarkedMethod>>isMarkedForUnwindInAction
       "Answer true if method is marked for unwinding in action."

       ^markType == #unwindInAction
       and I can spell practice ;)


       HTH
       Eliot


           Cheers,
             - Andreas

2010/3/4 Levente Uzonyi <[hidden email]>:
> On Thu, 4 Mar 2010, Igor Stasenko wrote:
>
>> 2010/3/4 Levente Uzonyi <[hidden email]>:
>>>
>>> On Thu, 4 Mar 2010, Igor Stasenko wrote:
>>>
>>>> On 4 March 2010 01:56, Nicolas Cellier
>>>> <[hidden email]> wrote:
>>>>>
>>>>> 2010/3/4 Levente Uzonyi <[hidden email]>:
>>>>>>
>>>>>> On Thu, 4 Mar 2010, Nicolas Cellier wrote:
>>>>>>
>>>>>>> 2010/3/3 Levente Uzonyi <[hidden email]>:
>>>>>>>>
>>>>>>>> On Wed, 3 Mar 2010, Andreas Raab wrote:
>>>>>>>>
>>>>>>>>> On 3/3/2010 2:07 PM, Levente Uzonyi wrote:
>>>>>>>>>>
>>>>>>>>>> On Wed, 3 Mar 2010, Igor Stasenko wrote:
>>>>>>>>>>>
>>>>>>>>>>> i don't get it. Just before that, you said: ' I'd expect it to be
>>>>>>>>>>> evaluated no matter what happens.' ?
>>>>>>>>>>> But now you saying that it may not be executed in some conditions
>>>>>>>>>>> (when user pressing abandon button, causing process to be
>>>>>>>>>>> terminated).
>>>>>>>>>>
>>>>>>>>>> It's simple: don't terminate process X from another process if
>>>>>>>>>> process
>>>>>>>>>> X
>>>>>>>>>> is executing a termiation block (aka #ensure: block). Or if you
>>>>>>>>>> terminate it, make sure that the execution of the block will
>>>>>>>>>> continue
>>>>>>>>>> somehow (I don't care how).
>>>>>>>>>
>>>>>>>>> You're missing Igors point which is that in his example the halt /
>>>>>>>>> Transcript *was* in the ensure block and as a result you're
>>>>>>>>> contradicting
>>>>>>>>> yourself here. Let's go back to Igor's example:
>>>>>>>>>
>>>>>>>>> [self boom ] ensure: [ self halt. Transcript show: 'boom']
>>>>>>>>>
>>>>>>>>> The halt is inside the ensure block. If you terminate the process
>>>>>>>>> from
>>>>>>>>> the
>>>>>>>>> debugger, it would be logical from your statement that the
>>>>>>>>> Transcript
>>>>>>>>> message would be executed - after all it's " executing a termiation
>>>>>>>>> block
>>>>>>>>> (aka #ensure: block)" and so it can't be terminated by your
>>>>>>>>> reasoning.
>>>>>>>>> However, when Igor was pointing this out you replied with "I didn't
>>>>>>>>> say
>>>>>>>>> that. I said evaluate it the same way as normal code." which is
>>>>>>>>> inconsistent
>>>>>>>>> with the other statement.
>>>>>>>>
>>>>>>>> That shows my lack of knowledge about how the debugger works.
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> I think every user of #ensure: expects that the termination blocks
>>>>>>>>>> are
>>>>>>>>>> executed even if the process which is executing the receiver of
>>>>>>>>>> #ensure:
>>>>>>>>>> is terminated. And it actually happens in all but this case.
>>>>>>>>>
>>>>>>>>> The question of terminating processes is always tricky. I don't
>>>>>>>>> think
>>>>>>>>> that
>>>>>>>>> your proposal would actually work in practice - it could easily
>>>>>>>>> result
>>>>>>>>> in
>>>>>>>>> processes that cannot be terminated due to a simple bug in an
>>>>>>>>> ensure
>>>>>>>>> block.
>>>>>>>>> Personally, I'd rather say that the more useful behavior would be
>>>>>>>>> something
>>>>>>>>> along the lines of saying that process termination either skips the
>>>>>>>>> current
>>>>>>>>> ensure block (assuming there's a bug and it should get the heck out
>>>>>>>>> of
>>>>>>>>> it
>>>>>>>>> but try to evaluate the remaining ones) or that there need to be
>>>>>>>>> two
>>>>>>>>> terminations - one that is 'soft' and won't allow ensure blocks to
>>>>>>>>> be
>>>>>>>>> skipped and one that is 'hard' (kill -9 hard) and just ignores all
>>>>>>>>> the
>>>>>>>>> ensure blocks.
>>>>>>>>
>>>>>>>> I'm only saying that normal usage (aka #terminate) shouldn't do
>>>>>>>> unexpected
>>>>>>>> things like this.
>>>>>>>> If you read the comment of Process >> #terminate, you may assume
>>>>>>>> that
>>>>>>>> #ensure: and #ifCurtailed: blocks will be excuted even if you use
>>>>>>>> #terminate, but that's not true.
>>>>>>>>
>>>>>>>> "Stop the process that the receiver represents forever.  Unwind to
>>>>>>>> execute
>>>>>>>> pending ensure:/ifCurtailed: blocks before terminating."
>>>>>>>>
>>>>>>>>
>>>>>>>> Levente
>>>>>>>>
>>>>>>>
>>>>>>> The only way I see to solve your problem would be to execute the
>>>>>>> unwind block in another process...
>>>>>>> Quite technical and costly !
>>>>>>
>>>>>> It's our problem. Just look at the senders of #ensure: and imagine
>>>>>> what
>>>>>> will
>>>>>> happen if the termination block is not evaluated.
>>>>>> I think there's another way (though it might be my lack of knowledge
>>>>>> again).
>>>>>> After suspending the process which is about to be terminated we can
>>>>>> check if
>>>>>> it's executing a termination block. It it's not, we are safe to
>>>>>> continue
>>>>>> the
>>>>>> termination, otherwise we can do something else which ensures that the
>>>>>> termination block is evaluated.
>>>>>>
>>>>>
>>>>> Maybe...
>>>>> Unfortunately, you did not tell how you will distinguish well behaved
>>>>> unwind-blocks from Igor's example...
>>>>>
>>>>
>>>> Yes, then what prevents me from writing:
>>>>
>>>> [ [ ] ensure: [ self doCrazyThings ] ] fork.
>>>
>>> What prevents you from writing: Object superclass: Object. ?
>>> Nothing, but you don't do that, do you?
>>>
>> So, why at all, you care about using #ensure: then? If you putting
>> everything up to the hands of developer,
>> then obviously you won't need to use this message, because you always
>> know that you're running a reliable code which
>> will always let you to run your things in the end. :)
>>
>>>>
>>>> and now given assumption that any code which placed inside ensure
>>>> block should always run to the end, without chances being terminated,
>>>> will have ill side effects.
>>>
>>> You can terminate it (maybe not the usual way).
>>>
>> that's the point. Why do we need two (or more) ways to terminate a
>> process?
>>
>>>> The #ensure: means, that interpreter will have a chance to enter that
>>>> block eventually, but it should not mean that it will keep running the
>>>> code there until normal or non-local return from that block.
>>>
>>> Then it doesn't ensure anything at all, so it should be called
>>> #tryToEvaluateThisToo:.
>>>
>> in fact, this is the actual behavior :)
>> If i press the power button on your PC, or plug out the power cord,
>> any #ensure: blocks will have no chances to run either way.
>> So, relying on something, which is not reliable is a fallacy :)
>
> Here's a simple example (replace file with any external resource):
> My process opened a file, a termination block will close it if it's
> evaluated. If I send #terminate to the process I expect my file to be
> closed, so I won't leak a file descriptor.
> If the file can't be closed (aka the termination block raised an error) then
> there's a serious problem. It doesn't really matter what happens then.
>
> But I don't have to try to convince you anymore, because Andreas is about to
> solve the issue.
>

I'm not trying to convince anyone, i just wanted to show you that
there is no good solution in that plane.
More workarounds means more code to run (and makes things more complex, btw).
But you will be still unsafe.You will be safe, once you stop relying
on #ensure: in your code and use different approach.

As for your example: use weak finalizer to close your file.
This will make sure that no matter what were happen, you wont leave
the file open. Working with external resources is a pain. But lets try
to not poison ourselves with manual resource management, which comes
from C world.

>
> Levente
>
>>
>>>
>>> Levente
>>>
>>>> Othewise, you losing a way to terminate unwanted, ill-behaved process.
>>>>
>>>>> Nicolas
>>>>>
>>>>>>
>>>>>> Levente
>>>>>>
>>>>>>>
>>>>>>> Nicolas
>>>>>>>
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>>  - Andreas
>>>>>>>>>
>>>>
>>>> --
>>>> Best regards,
>>>> Igor Stasenko AKA sig.
>>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>> --
>> Best regards,
>> Igor Stasenko AKA sig.
>>
>
>
>
>

--

Best regards,
Igor Stasenko AKA sig.