> On 9 Aug 2016, at 00:03, Nicolai Hess <[email protected]> wrote: > > > > 2016-08-08 17:35 GMT+02:00 Max Leske <[email protected] > <mailto:[email protected]>>: > >> On 8 Aug 2016, at 17:15, Nicolai Hess <[email protected] >> <mailto:[email protected]>> wrote: >> >> >> >> 2016-08-08 15:26 GMT+02:00 Max Leske <[email protected] >> <mailto:[email protected]>>: >> >>> On 8 Aug 2016, at 15:00, Dale Henrichs <[email protected] >>> <mailto:[email protected]>> wrote: >>> >>> Max, >>> >>> Thanks for looking into this. >>> >>> Do you think that this bug will be fixed in Pharo5.0? When I'm debugging >>> the tests with this fatal pattern, my only recourse is to start and stop >>> images between test runs ... >>> >>> >> >> If this is indeed a bug in the termination logic then yes, it will be ported >> to Pharo 5 since it’s critical. >>> The side effect of not running ensure blocks in tests is that a SharedQueue >>> gets stuck waiting on an empty queue and when I interrupt that process (and >>> get another debugger that is closed) I end up with the Empty Debugger >>> problem where the debuggers have decide to stop working ... I saw that >>> there was logic in Process>>terminate involved in dealing with processes >>> running in critical blocks and that logic might be faulty as well ... >>> >>> >> >> Thanks for the details. >> >> >> Hey Max, >> >> I looked at >> Process>>#terminate >> and I think the problem is that it sets isTerminating := true, but later for >> the "Unwind error during termination", it spaws a new debugger for this >> process. >> And if we now again press "Abondand" we don't try to terminate this process >> (and don't call the inner ensure-block) because isTerminating is already >> true. > > Thanks Nicolai, > > I can’t look at the code at the moment, but as far as I recall, sending > #terminate twice should signal a warning. So if what you’re saying is true, > I’d expect to see a debugger opened with that warning. I also think that even > if there was an unwind error, the process that is already terminating the > process within termination should be the only process to continue > termination. In other terms: only a single process should ever execute > termination for a given process. But that’s an educated guess, as I don’t > know how the exception handling is implemented for unwind errors. > > If it’s indeed how you describe, then I think the proper way to terminate the > process in case of an unwind error, would be to pop the contexts up to the > next unwind handler and execute that. #terminate should not be sent multiple > times. > > > There are actually two ensure-blocks involved. > If we "Abandon" the first debugger window, the ensure block of > TestCase>>#runCase is called, which in turn throws an error from > BugTestCase>>#tearDown, so we now have an exception during > process terminations unwind operation. That is why the second debugger > windonw shows "Unwind error during termination" instead of "Error:teardown". > If we new "ABandon" this second debugger window, we try to terminate a > process that is within its terminate-and-unwind operation.
Ok. Thanks for the analysis. > > Cheers, > Max > >> >> >> >>> Dale >>> >>> On 8/8/16 12:11 AM, Max Leske wrote: >>>> Thanks Nicolai. >>>> >>>> I’ve opened an issue on phogbugz: >>>> https://pharo.fogbugz.com/f/cases/18885/Ensure-blocks-in-test-tear-down-not-always-executed >>>> >>>> <https://pharo.fogbugz.com/f/cases/18885/Ensure-blocks-in-test-tear-down-not-always-executed>. >>>> >>>> >>>>> On 8 Aug 2016, at 09:03, Nicolai Hess <[email protected] >>>>> <mailto:[email protected]>> wrote: >>>>> >>>>> >>>>> >>>>> 2016-08-08 8:52 GMT+02:00 Max Leske <[email protected] >>>>> <mailto:[email protected]>>: >>>>> Wow, that’s pretty bad. The process termination logic should be taking >>>>> care of the ensure blocks. Unfortunately I can’t run any Pharo images at >>>>> the moment but if there’s something wrong with process termination then >>>>> it’s likely that me or Ben made some mistake. Could someone please rerun >>>>> Dale’s test scenario with the original process termination code (e.g. >>>>> from Pharo 3) and report the results? >>>>> >>>>> Yes, test runs fine in pharo 30864 (halts in the ensure block) >>>>> >>>>> >>>>> Cheers, >>>>> Max >>>>> >>>>> >>>>> > On 8 Aug 2016, at 03:25, Dale Henrichs >>>>> > <[email protected] >>>>> > <mailto:[email protected]>> wrote: >>>>> > >>>>> > While attempting to characterize the "Empty Debugger" problem that I've >>>>> > recently reported[1], I found that ensure blocks in TestCase>>teardown >>>>> > methods are not run if an Error is signaled while executing the code >>>>> > protected by the ensure block ... when the test itself brings up a >>>>> > debugger --- now that is a mouthful :) ... but a situation that >>>>> > commonly occurs ... >>>>> > >>>>> > I've attached a fileIn with a very simple reproduction of this problem. >>>>> > After filing in the code, execute the following: >>>>> > >>>>> > BugTestCase debug: #test. >>>>> > >>>>> > Abandon the first halt -- this is the halt in the test. Next a debugger >>>>> > is brought up on the error from inside the ensure block in the tearDown >>>>> > method: >>>>> > >>>>> > tearDown >>>>> > [ self error: 'teardown' ] >>>>> > ensure: [ >>>>> > "Imagine that something important NEEDED to be done here" >>>>> > self halt ]. >>>>> > super tearDown. >>>>> > >>>>> > Abandon the error and the `self halt` in the ensure block is not run ... >>>>> > >>>>> > If you take the `self halt` out of the test method itself, the `self >>>>> > halt` in the ensure block _is_ run ... >>>>> > >>>>> > This does not directly lead to the Empty Debugger, but when the ensure >>>>> > blocks called during teardown are not run, a resource is not cleaned up >>>>> > properly and that leads to SharedQueue hanging ... when I interrupt the >>>>> > SharedQueue, I believe that this leads to the "Empty Debugger" a >>>>> > separate, but more nasty problem ... >>>>> > >>>>> > Dale >>>>> > >>>>> > [1] http://forum.world.st/Re-Empty-debugger-Pharo-5-0-td4909911.html >>>>> > <http://forum.world.st/Re-Empty-debugger-Pharo-5-0-td4909911.html> >>>>> > >>>>> > <BugTestCase.st <http://bugtestcase.st/>>
