ExceptionTester>>simpleOuterTest fails (3.6beta 5395)
Reproduce with (ExceptionTester is part of the BaseImage Test Package):
ExceptionTester new runTest: #simpleOuterTest) suiteLog
Here's the test:
simpleOuterTest "uses #resume"
[[self doSomething. MyTestNotification signal. self doSomethingElse] on: MyTestNotification do: [:ex | ex outer]] on: MyTestNotification do: [:ex | self doYetAnotherThing. ex resume]
simpleOuterTest _still_ fails and we're at the end of beta for the release after the one it was reported from. Pretty bad. It means that anyone raising an exception is not going to get what they expect.
Who is caring for exceptions these days?
The SUnit test for Exception>>outer fails.
This change set provides a fix for Exception>>resumeUnchecked: that corrects the problem.
An additional test case is added for #testDoubleOuter that describes expected behavior for a two-level nested outer exception.
All ExceptionsTests are now green.
Notes for reviewers:
1) I am not an expert on exceptions. This is a simple fix to one method, and it produces the right results, but it needs a good review by someone who would understand any possible side effects.
2) The #testDoubleOuter test is new. It describes my understanding of how a doubly-nested outer exception should work, but please look at it carefully to make sure I've got it right.
Dave
Well bravo to David for looking into this - it hurts my head horribly to mess with exceptions and I swore I'd never do so again after spending eighteen months trying to persuade people that having the support prims in the VM made sense.
However, this is an IMPORTANT bug. If exceptions and unwinds are to be used they have to be dealt with correctly.
I hope that David's fix is not the correct one since it changes #returnUnchecked: to always return from the signalContext whereas the comment claims it should return from somewhere else if the method is called after an #outer. Part of the trouble here is a very unclear comment. I'm hoping that changing #outer will be a solution - but since it isn't actually used anywhere except in the test that was failing it's a bit hard to see what it is meant to do. Another lousy comment.
Whilst digging around I was also startled by the current implementation of #aboutToReturn:through: which really doesn't look as if it could possibly work. I'm reasonably convinced that it shouldn't involve #terminateTo: until after the unwinding is completed but I could be wrong. Can anyone explain the comment in ContextPart>return: ? If nothing else it's a bit strange to not make use of the VM provided initial context.
I suggest that a key part of getting this sorted out is a good number of tests that actually exercise the system thoroughly. The current exception testing is just sixteen tests which surely can't be covering all the kinds of use (and abuse) that exceptions are subjected to?
tim -- Tim Rowledge, tim@sumeru.stanford.edu, http://sumeru.stanford.edu/tim Drugs may lead to nowhere, but at least it's the scenic route.
Closed in favour of the later thread on the same subject
squeak-dev@lists.squeakfoundation.org
-
lewis＠mail.msen.com -
Marcus Denker -
Tim Rowledge -
tim＠sumeru.stanford.edu