[Pharo-project] Another finalization concern: error handling

Henrik Johansen henrik.s.johansen at veloxit.no
Tue Oct 12 08:33:19 EDT 2010

On Oct 12, 2010, at 2:00 42PM, Igor Stasenko wrote:

> On 12 October 2010 14:25, Henrik Johansen <henrik.s.johansen at veloxit.no> wrote:
>> On Oct 11, 2010, at 10:17 18PM, Igor Stasenko wrote:
>>> On 11 October 2010 22:49, Schwab,Wilhelm K <bschwab at anest.ufl.edu> wrote:
>>>> Sig,
>>>> The most important words in there are "critical section."  Carry on :)
>>> Oh, please. This is not too hard to code.
>>> My mind rolling around following choice(s)  (there may be others i don't see).
>>> What would be a proper way to handle error during #finalize.
>>> [ executor finalize ] on: Error do: [:ex |
>>>  self handleFinalizationError: ex  "where self is registry"
>>> ].
>>> or:
>>> [ executor finalize ] on: Error do: [:ex |
>>>  executor handleFinalizationError: ex
>>> ].
>>> of course, i should catch this error in test, so i can verify that:
>>> a) test is get notified upon synthetically made error
>>> b) no matter what i do inside error handler (up to 'Processor
>>> activeProcess terminate'), a finalization process continues working
>>> (or restarts without losing remainder of executors).
>> I agree it should be the executor doing handling.
>> However, if you get to this point, the executor already had the chance to do so in the finalize method itself, so I don't really see the point of sending it back..
> well,  its easier to add a default error handler in Object class, so
> any executor will have a default handler,
> and then some specific one may choose to override it, without putting
> a boilerplate of [ ] on: .. do: [] in every #finalize method.
> Besides, its natural , when you writing a code, you always expect that
> it will work w/o errors,
> and even if it fails, then in some concrete place, which means you
> usually not wrapping whole thing with exception handler.
> This leaves a chance to make another mistakes, like 'hey, despite i
> put error handler in my #finalize, something is still hangs
> finalization process'.

At which point I'd go "Fix the damn handlers in the finalize method" rather than add more indirection ;) KISS.
You still have to write the handling if you do encounter an error, whether that initial error is #dnu handleFinalizationError:, or the actual error is of little consequence.  
What we want away from, is the default case now where an unhandled error is assumed to be inconsequential and simply skipped over by default

>> Personally, I feel the only promises FinalizationRegistry should make wrt. to errors during finalization, is it should never lead to:
>> - termination of the (or a) finalization process,
>> - discarding another finalization action.
> Yes this is the main intent.
>> IE I am perfectly fine with an error here being thrown in your face (hopefully in a somewhat debuggable way, unlike what happened in your first example when removing the error-swallowing), as you've already passed the point a correction should be made, and there's nothing more intelligible to do than making sure everything keeps working, and inform of the error.
>> (Pretty sure the error swallowing was a quick hack for keeping everything working in the first place ;) )
> What i like in doing #finalize in forked process, that it doesn't
> matter where you screwed up: in finalization code or even in error
> handling code, you still don't have a chance to damage the system. And
> moreover, you can open debugger and debug it.
> Spice must flooow :)
How about something like: (warning, untested code)

WeakArray >> finalizationProcess

	^[[[true] whileTrue:
		[FinalizationSemaphore wait.
		FinalizationLock critical:
			[FinalizationDependents do:
				[:weakDependent |
				weakDependent ifNotNil:
					[weakDependent finalizeValues]]]]] 
		ifCurtailed: [self startFinalizationProcess]] newProcess priority:  Processor userInterruptPriority

WeakArray >>startFinalizationProcess

	FinalizationSemaphore := Smalltalk specialObjectsArray at: 42.
	FinalizationDependents ifNil: [FinalizationDependents := WeakArray new: 10].
	FinalizationLock := Semaphore forMutualExclusion.
	FinalizationProcess := self finalizationProcess.
	FinalizationProcess resume

WeakArray >>restartFinalizationProcess
	"Killing the current Finalization process will start a new one"
	FinalizationProcess terminate

WeakRegistry finalizeValues:

self finalize: finiObjects

WeakArray >> finalize: finalizationObjects
	[[finalizationObjects isEmpty] 
		whileFalse: [finalizationObjects removeFirst finalize]]
		 on: Error
		do: [:error | 
			self finalize: (finalizationObjects).
			error pass].

The last has some issues of course, if more than one error is encountered, the first ones won't be passed if the last ones are terminated.
Changing it to fork of the error block would mean having to change finalizationObjects to something that is threadsafe rather than OrderedCollection though.


More information about the Pharo-dev mailing list