About the infinite debugger
Hi all, during today's sprint we have been working with lots of people on the infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We have checked the emails sent in the latest month. Then, together with Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis all day. We have been also comparing the debuggers code between pharo 3/4 (where the bug was is present) and pharo 7, but this was not necessarily straight forward as the code is not the same and there is no easy diff... AAAAND, we have found that the problem may come from a wrong pragma marker. Just removing that pragma gives us back the same behaviour as in Pharo 3/4. :D https://github.com/pharo-project/pharo/pull/1621 I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look? Thanks all for the fish, Guille
Hi, Normally the pull request should break the exception tests, especially the ones with nested handler, since they are the reason 199 marked signalling contexts were introduced. Looking quickly this test in latest Pharo 7 for example: ExceptionTests>>testHandlerContext fails with the pull request. Since the pull request changes a green test into a red test, I don't understand why you say "we unfortunately don't have tests for it". I don't understand either why you say this pragma is "wrong", so I can't help with that either. Best, On Fri, Jun 29, 2018 at 4:48 PM, Guillermo Polito <guillermopolito@gmail.com
wrote:
Hi all,
during today's sprint we have been working with lots of people on the infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We have checked the emails sent in the latest month. Then, together with Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis all day. We have been also comparing the debuggers code between pharo 3/4 (where the bug was is present) and pharo 7, but this was not necessarily straight forward as the code is not the same and there is no easy diff...
AAAAND, we have found that the problem may come from a wrong pragma marker. Just removing that pragma gives us back the same behaviour as in Pharo 3/4. :D
https://github.com/pharo-project/pharo/pull/1621
I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Thanks all for the fish, Guille
-- Clément Béra https://clementbera.github.io/ https://clementbera.wordpress.com/
On Fri, Jun 29, 2018 at 5:54 PM Clément Bera <bera.clement@gmail.com> wrote:
Hi,
Normally the pull request should break the exception tests, especially the ones with nested handler, since they are the reason 199 marked signalling contexts were introduced
Yes, but it's ok that on:do: is marked but why evaluateSignal:? Do you know?
.
Looking quickly this test in latest Pharo 7 for example: ExceptionTests>>testHandlerContext fails with the pull request.
Since the pull request changes a green test into a red test, I don't understand why you say "we unfortunately don't have tests for it".
Because I haven't see those. I was looking for tests about #findNextHandlerContext, #findNextHandlerOrSignalingContext and I found nothing. But if we have some tests much better!
I don't understand either why you say this pragma is "wrong", so I can't help with that either.
Well, I don't know if I have to repeat myself... This is apparently provoking the infinite debugger issue. This pragma was not there in Pharo3, and its not clear why a method that is not an exception handler is marked as an exception handler. Is that clear enough? For more info you can read the issue.
Best,
On Fri, Jun 29, 2018 at 4:48 PM, Guillermo Polito < guillermopolito@gmail.com> wrote:
Hi all,
during today's sprint we have been working with lots of people on the infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We have checked the emails sent in the latest month. Then, together with Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis all day. We have been also comparing the debuggers code between pharo 3/4 (where the bug was is present) and pharo 7, but this was not necessarily straight forward as the code is not the same and there is no easy diff...
AAAAND, we have found that the problem may come from a wrong pragma marker. Just removing that pragma gives us back the same behaviour as in Pharo 3/4. :D
https://github.com/pharo-project/pharo/pull/1621
I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Thanks all for the fish, Guille
-- Clément Béra https://clementbera.github.io/ https://clementbera.wordpress.com/
-- Guille Polito Research Engineer Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - *http://www.cnrs.fr <http://www.cnrs.fr>* *Web:* *http://guillep.github.io* <http://guillep.github.io> *Phone: *+33 06 52 70 66 13
On 29 June 2018 at 22:48, Guillermo Polito <guillermopolito@gmail.com> wrote:
Hi all,
during today's sprint we have been working with lots of people on the infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We have checked the emails sent in the latest month. Then, together with Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis all day. We have been also comparing the debuggers code between pharo 3/4 (where the bug was is present) and pharo 7, but this was not necessarily straight forward as the code is not the same and there is no easy diff...
AAAAND, we have found that the problem may come from a wrong pragma marker. Just removing that pragma gives us back the same behaviour as in Pharo 3/4. :D
https://github.com/pharo-project/pharo/pull/1621
I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Thanks all for the fish, Guille
I'm just about to fly out for a family vacation, so I'm not sure when I'll get a chance to have a good look. But I read the issue on Fogbugz and it echos some things I've been thinking recently, though I hadn't come close to that particular fix. I think my following comments are probably unrelated, but just sharing some thoughts that crossed my mind when I was looking into my own similar trouble. This is an area I'm not clear on and would like to understand better... When the UI thread (UI1) needs to be debugged (e.g. code run from Playground, or a test) and is suspended and a new UI thread (UI2) is started, which thread does the debugger run in? I guess it must run in UI2 but sometimes I got the vague feeling some parts of it were caught in UI1 and affected by UI1 being suspended. But perhaps I got the wrong impression - its a complex interplay to follow. The other vague notion I had was when thread UI1 is resumed, it seems to replace thread UI2 where the World keeps its, and wondered if <StepOver> on the suspended-debug-UI1 thread somehow made it "resume" and so replace UI2 as the main thread, but then UI1 gets suspended again and the display freezes. Thanks for everyones efforts on this. cheers -ben
On Fri, Jun 29, 2018 at 5:58 PM Ben Coman <btc@openinworld.com> wrote:
On 29 June 2018 at 22:48, Guillermo Polito <guillermopolito@gmail.com> wrote:
Hi all,
during today's sprint we have been working with lots of people on the infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We have checked the emails sent in the latest month. Then, together with Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis all day. We have been also comparing the debuggers code between pharo 3/4 (where the bug was is present) and pharo 7, but this was not necessarily straight forward as the code is not the same and there is no easy diff...
AAAAND, we have found that the problem may come from a wrong pragma marker. Just removing that pragma gives us back the same behaviour as in Pharo 3/4. :D
https://github.com/pharo-project/pharo/pull/1621
I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Thanks all for the fish, Guille
I'm just about to fly out for a family vacation, so I'm not sure when I'll get a chance to have a good look. But I read the issue on Fogbugz and it echos some things I've been thinking recently, though I hadn't come close to that particular fix.
I think my following comments are probably unrelated, but just sharing some thoughts that crossed my mind when I was looking into my own similar trouble. This is an area I'm not clear on and would like to understand better...
When the UI thread (UI1) needs to be debugged (e.g. code run from Playground, or a test) and is suspended and a new UI thread (UI2) is started, which thread does the debugger run in?
Yes
I guess it must run in UI2 but sometimes I got the vague feeling some parts of it were caught in UI1 and affected by UI1 being suspended.
But perhaps I got the wrong impression - its a complex interplay to follow.
Nope.. Actually, UI2 will simulate code of UI1 as it was running in UI1. There is some machinery (look at #effectiveProcess) so that Processor activeProcess yields UI1 when stepping over UI2 code.
The other vague notion I had was when thread UI1 is resumed, it seems to replace thread UI2 where the World keeps its, and wondered if <StepOver> on the suspended-debug-UI1 thread somehow made it "resume" and so replace UI2 as the main thread, but then UI1 gets suspended again and the display freezes.
Thanks for everyones efforts on this. cheers -ben
-- Guille Polito Research Engineer Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - *http://www.cnrs.fr <http://www.cnrs.fr>* *Web:* *http://guillep.github.io* <http://guillep.github.io> *Phone: *+33 06 52 70 66 13
Great news! I hope this fixes it once and for all. For some reason this bug bit me several times in several Pharo releases. Thank you all for your dedication. On 29/06/2018 11:48, Guillermo Polito wrote:
Hi all,
during today's sprint we have been working with lots of people on the infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We have checked the emails sent in the latest month. Then, together with Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis all day. We have been also comparing the debuggers code between pharo 3/4 (where the bug was is present) and pharo 7, but this was not necessarily straight forward as the code is not the same and there is no easy diff...
AAAAND, we have found that the problem may come from a wrong pragma marker. Just removing that pragma gives us back the same behaviour as in Pharo 3/4. :D
https://github.com/pharo-project/pharo/pull/1621
I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Thanks all for the fish, Guille
-- Esteban A. Maringolo
Hi Guille,
On Jun 29, 2018, at 7:48 AM, Guillermo Polito <guillermopolito@gmail.com> wrote:
Hi all,
during today's sprint we have been working with lots of people on the infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We have checked the emails sent in the latest month. Then, together with Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis all day. We have been also comparing the debuggers code between pharo 3/4 (where the bug was is present) and pharo 7, but this was not necessarily straight forward as the code is not the same and there is no easy diff...
This is frustrating. I canât see the issue cuz I canât login to fogbugz. Having to login to read an issue is a major flaw. I can see it makes sense for submitting, but fir merely browsing it should be unacceptable. That said... The pragma <primitive: 199> actually sets the primitive number in the method, so it it not merely a pragma; it alters bits in the method that the VM uses to search for handler contexts. So why one would do that for evaluateSignal: makes no sense to me. The primitive should be set only in on:do: or something very similar (for example one could imagine adding on:or:do: instead of using , to construct an ExceptionSet). So I think removing it from evaluateSignal: is definitely the right thing to do. As far as tests for findNextHandlerFrom:, this is tested implicitly by any nested exception test so I expect you have several tests affected. Clément points to a test that fails when not including <primitive: 199> in evaluateSignal: so more investigation is necessary. Difficult to do while bugs are hidden in fogbugz. When are they going to migrate the github where they belong?
AAAAND, we have found that the problem may come from a wrong pragma marker. Just removing that pragma gives us back the same behaviour as in Pharo 3/4. :D
https://github.com/pharo-project/pharo/pull/1621
I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Thanks all for the fish, Guille
On 30 Jun 2018, at 02:56, Eliot Miranda <eliot.miranda@gmail.com> wrote:
Hi Guille,
On Jun 29, 2018, at 7:48 AM, Guillermo Polito <guillermopolito@gmail.com <mailto:guillermopolito@gmail.com>> wrote:
Hi all,
during today's sprint we have been working with lots of people on the infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/ <https://pharo.fogbugz.com/f/cases/22085/>). We have checked the emails sent in the latest month. Then, together with Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis all day. We have been also comparing the debuggers code between pharo 3/4 (where the bug was is present) and pharo 7, but this was not necessarily straight forward as the code is not the same and there is no easy diff...
This is frustrating. I canât see the issue cuz I canât login to fogbugz. Having to login to read an issue is a major flaw. I can see it makes sense for submitting, but fir merely browsing it should be unacceptable. That said...
The pragma <primitive: 199> actually sets the primitive number in the method, so it it not merely a pragma; it alters bits in the method that the VM uses to search for handler contexts. So why one would do that for evaluateSignal: makes no sense to me. The primitive should be set only in on:do: or something very similar (for example one could imagine adding on:or:do: instead of using , to construct an ExceptionSet). So I think removing it from evaluateSignal: is definitely the right thing to do.
As far as tests for findNextHandlerFrom:, this is tested implicitly by any nested exception test so I expect you have several tests affected. Clément points to a test that fails when not including <primitive: 199> in evaluateSignal: so more investigation is necessary. Difficult to do while bugs are hidden in fogbugz. When are they going to migrate the github where they belong?
Yes, the idea was to move the issue tracker to GitHub⦠but after the release is done (as it requires changing again some things). The closed-ness of fugbugz is really annoying. Marcus
On 06/29/2018 07:48 AM, Guillermo Polito wrote:
Hi all,
during today's sprint we have been working with lots of people on the infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We have checked the emails sent in the latest month. Then, together with Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis all day. We have been also comparing the debuggers code between pharo 3/4 (where the bug was is present) and pharo 7, but this was not necessarily straight forward as the code is not the same and there is no easy diff...
AAAAND, we have found that the problem may come from a wrong pragma marker. Just removing that pragma gives us back the same behaviour as in Pharo 3/4. :D
https://github.com/pharo-project/pharo/pull/1621
I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Hi Guille, I'm just back from vacation last week, and about to go on vacation for another week, but I'll see what I can see. About the primitive pragmas for context-marking, I think some of those were changed for the exception handling fix that Andres and I did a few years back, so *could* be involved in this. I'd hate to see regression in the exception handling in an attempt to fix this bug. Regards, -Martin
On 07/03/2018 05:02 PM, Martin McClure wrote:
On 06/29/2018 07:48 AM, Guillermo Polito wrote:
I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Hi Guille, I'm just back from vacation last week, and about to go on vacation for another week, but I'll see what I can see. About the primitive pragmas for context-marking, I think some of those were changed for the exception handling fix that Andres and I did a few years back, so *could* be involved in this. I'd hate to see regression in the exception handling in an attempt to fix this bug.
After a look at at the pull request, I'm quite sure that removing the prim 199 marker is the wrong thing to do. Thanks, Pablo, for restoring it! The start of execution of exception handlers must be marked in order for #findNextHandlerContext to work correctly. If these contexts are not marked, exceptions signaled from inside an exception handler can find the wrong handler. See the code and comment in #findNextHandlerContext. I'm afraid that I cannot immediately help with the debugger problem, since I don't know the debugger nearly as well as I do the exception handling code, and I'm going on vacation for a week in 20 minutes. :-) Perhaps when I get back I can take a look at it if it is still a problem by then. Regards, -Martin
bump
Am 04.07.2018 um 02:28 schrieb Martin McClure <martin@hand2mouse.com>:
On 07/03/2018 05:02 PM, Martin McClure wrote:
On 06/29/2018 07:48 AM, Guillermo Polito wrote: I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Hi Guille, I'm just back from vacation last week, and about to go on vacation for another week, but I'll see what I can see. About the primitive pragmas for context-marking, I think some of those were changed for the exception handling fix that Andres and I did a few years back, so *could* be involved in this. I'd hate to see regression in the exception handling in an attempt to fix this bug.
After a look at at the pull request, I'm quite sure that removing the prim 199 marker is the wrong thing to do. Thanks, Pablo, for restoring it! The start of execution of exception handlers must be marked in order for #findNextHandlerContext to work correctly. If these contexts are not marked, exceptions signaled from inside an exception handler can find the wrong handler. See the code and comment in #findNextHandlerContext.
I'm afraid that I cannot immediately help with the debugger problem, since I don't know the debugger nearly as well as I do the exception handling code, and I'm going on vacation for a week in 20 minutes. :-) Perhaps when I get back I can take a look at it if it is still a problem by then.
Regards, -Martin
Guys - this really needs attention - Iâve spend hours now trying to debug some code and most of it is in closing infinite debuggers. It makes a mockery of our tagline - âawesome debuggingâ. And the extra irony is that Iâm debugging some file path stuff for exercism, to make it easier for hopefully more people to learn Pharo. How can we get this fixed? Tim
On 2 Aug 2018, at 21:44, Norbert Hartl <norbert@hartl.name> wrote:
bump
Am 04.07.2018 um 02:28 schrieb Martin McClure <martin@hand2mouse.com>:
On 07/03/2018 05:02 PM, Martin McClure wrote:
On 06/29/2018 07:48 AM, Guillermo Polito wrote: I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Hi Guille, I'm just back from vacation last week, and about to go on vacation for another week, but I'll see what I can see. About the primitive pragmas for context-marking, I think some of those were changed for the exception handling fix that Andres and I did a few years back, so *could* be involved in this. I'd hate to see regression in the exception handling in an attempt to fix this bug.
After a look at at the pull request, I'm quite sure that removing the prim 199 marker is the wrong thing to do. Thanks, Pablo, for restoring it! The start of execution of exception handlers must be marked in order for #findNextHandlerContext to work correctly. If these contexts are not marked, exceptions signaled from inside an exception handler can find the wrong handler. See the code and comment in #findNextHandlerContext.
I'm afraid that I cannot immediately help with the debugger problem, since I don't know the debugger nearly as well as I do the exception handling code, and I'm going on vacation for a week in 20 minutes. :-) Perhaps when I get back I can take a look at it if it is still a problem by then.
Regards, -Martin
Hi, i had no answer to my comments on fogbuz (one of the first) so i assumed it was not a good idea. In the usecases i used to reproduce the bug, replacing "ex pass" by "ex debug" in runCaseForDebug:solved the problem. See the analysis on fogbuz. Did not have any side effect, but i do not know enough about the system and maybe it does. I think i already asked about that somewhere (here or on discord) but no answers. But maybe it is wrong to do that? I'd be happy to have feedback on that. Steven. Le 2018-08-05 22:27, Tim Mackinnon a écrit :
Guys - this really needs attention - I've spend hours now trying to debug some code and most of it is in closing infinite debuggers. It makes a mockery of our tagline - "awesome debugging". And the extra irony is that I'm debugging some file path stuff for exercism, to make it easier for hopefully more people to learn Pharo.
How can we get this fixed?
Tim
On 2 Aug 2018, at 21:44, Norbert Hartl <norbert@hartl.name> wrote:
bump
Am 04.07.2018 um 02:28 schrieb Martin McClure <martin@hand2mouse.com>:
On 07/03/2018 05:02 PM, Martin McClure wrote: On 06/29/2018 07:48 AM, Guillermo Polito wrote: I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look? Hi Guille, I'm just back from vacation last week, and about to go on vacation for another week, but I'll see what I can see. About the primitive pragmas for context-marking, I think some of those were changed for the exception handling fix that Andres and I did a few years back, so *could* be involved in this. I'd hate to see regression in the exception handling in an attempt to fix this bug.
After a look at at the pull request, I'm quite sure that removing the prim 199 marker is the wrong thing to do. Thanks, Pablo, for restoring it! The start of execution of exception handlers must be marked in order for #findNextHandlerContext to work correctly. If these contexts are not marked, exceptions signaled from inside an exception handler can find the wrong handler. See the code and comment in #findNextHandlerContext. I'm afraid that I cannot immediately help with the debugger problem, since I don't know the debugger nearly as well as I do the exception handling code, and I'm going on vacation for a week in 20 minutes. :-) Perhaps when I get back I can take a look at it if it is still a problem by then. Regards, -Martin
Hi Steven, The thing about your fix was mainly that it only worked for the case of running tests. We could however reproduce this bug from a playground too. At first, replacing #pass to #debug looked like a hack to me :) Just looking at the code of runCaseForDebug:, I felt the correct thing to do there was to use #pass (and let any potential caller handle the exception if he wants). Otherwise, we should be able to understand: - can #pass be used? is it buggy? - does it work on any case or it should be avoided in some cases? - and how can we fix it (or at least document it?)? But still, I think you were in the right direction too, because your fix shows a good intuition too: both #pass and #debug will do different things with the exception. - #pass will **resignal** it from the current context, - #debug will just open a debugger on it. So that means that probably there was a problem when the exception was handled in a calling context. On Mon, Aug 6, 2018 at 12:29 AM Steven Costiou <steven.costiou@kloum.io> wrote:
Hi,
i had no answer to my comments on fogbuz (one of the first) so i assumed it was not a good idea.
In the usecases i used to reproduce the bug, replacing "ex pass" by "ex debug" in runCaseForDebug:solved the problem. See the analysis on fogbuz.
Did not have any side effect, but i do not know enough about the system and maybe it does. I think i already asked about that somewhere (here or on discord) but no answers.
But maybe it is wrong to do that? I'd be happy to have feedback on that.
Steven.
Le 2018-08-05 22:27, Tim Mackinnon a écrit :
Guys - this really needs attention - I've spend hours now trying to debug some code and most of it is in closing infinite debuggers. It makes a mockery of our tagline - "awesome debugging". And the extra irony is that I'm debugging some file path stuff for exercism, to make it easier for hopefully more people to learn Pharo.
How can we get this fixed?
Tim
On 2 Aug 2018, at 21:44, Norbert Hartl <norbert@hartl.name> wrote:
bump
Am 04.07.2018 um 02:28 schrieb Martin McClure <martin@hand2mouse.com>:
On 07/03/2018 05:02 PM, Martin McClure wrote:
On 06/29/2018 07:48 AM, Guillermo Polito wrote: I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Hi Guille, I'm just back from vacation last week, and about to go on vacation for another week, but I'll see what I can see. About the primitive pragmas for context-marking, I think some of those were changed for the exception handling fix that Andres and I did a few years back, so *could* be involved in this. I'd hate to see regression in the exception handling in an attempt to fix this bug.
After a look at at the pull request, I'm quite sure that removing the prim 199 marker is the wrong thing to do. Thanks, Pablo, for restoring it! The start of execution of exception handlers must be marked in order for #findNextHandlerContext to work correctly. If these contexts are not marked, exceptions signaled from inside an exception handler can find the wrong handler. See the code and comment in #findNextHandlerContext.
I'm afraid that I cannot immediately help with the debugger problem, since I don't know the debugger nearly as well as I do the exception handling code, and I'm going on vacation for a week in 20 minutes. :-) Perhaps when I get back I can take a look at it if it is still a problem by then.
Regards, -Martin
-- Guille Polito Research Engineer Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - *http://www.cnrs.fr <http://www.cnrs.fr>* *Web:* *http://guillep.github.io* <http://guillep.github.io> *Phone: *+33 06 52 70 66 13
Hello, We have implemented a "patch" for Pharo 7, that is already integrated. I have created a slice to backport the "patch" to Pharo6. Basically, the previous situation is the following: - In the normal execution everything works, the problem is during the execution of over. - To implement over the debugger uses Context >> runUntilErrorOrReturnFrom: - This method adds a new context to handle exceptions in the stack trace. It takes the receiver and replaces the sender of the context with a new context handling the exceptions. - This error handling (using on:do:) handles UnhandledError and Halt. - When the MNU error gots to the nil context a UnhandledError is signal from the last signalling context of the MNU error. - This solution works correctly when it is not debugged when running a test. When we are running a tests there is a slight difference: - There is one more on:do: context that catches all the Errors. This on:do just do something and pass the exception. - As the UnhandledError is signal from the last signalling context (the one with the pass) it is not passing in the context inserted by runUntilErrorOrReturnFrom:, generating an infinite debugger as the exception is never catch. - This only happens with the MNU, as it retries to send the same message every time. I think the solution is to signal the UnhandledError always from the original signalling context. However, I am not sure to perform that change as it modifies the behaviour of exceptions and there are not proper tests to guarantee the expected behavior. I hope the problem is well explained, if it is not please ask me to clarify any point. Cheers, On Mon, Aug 6, 2018 at 9:59 AM Guillermo Polito <guillermopolito@gmail.com> wrote:
Hi Steven,
The thing about your fix was mainly that it only worked for the case of running tests. We could however reproduce this bug from a playground too.
At first, replacing #pass to #debug looked like a hack to me :) Just looking at the code of runCaseForDebug:, I felt the correct thing to do there was to use #pass (and let any potential caller handle the exception if he wants). Otherwise, we should be able to understand: - can #pass be used? is it buggy? - does it work on any case or it should be avoided in some cases? - and how can we fix it (or at least document it?)?
But still, I think you were in the right direction too, because your fix shows a good intuition too: both #pass and #debug will do different things with the exception. - #pass will **resignal** it from the current context, - #debug will just open a debugger on it.
So that means that probably there was a problem when the exception was handled in a calling context.
On Mon, Aug 6, 2018 at 12:29 AM Steven Costiou <steven.costiou@kloum.io> wrote:
Hi,
i had no answer to my comments on fogbuz (one of the first) so i assumed it was not a good idea.
In the usecases i used to reproduce the bug, replacing "ex pass" by "ex debug" in runCaseForDebug:solved the problem. See the analysis on fogbuz.
Did not have any side effect, but i do not know enough about the system and maybe it does. I think i already asked about that somewhere (here or on discord) but no answers.
But maybe it is wrong to do that? I'd be happy to have feedback on that.
Steven.
Le 2018-08-05 22:27, Tim Mackinnon a écrit :
Guys - this really needs attention - I've spend hours now trying to debug some code and most of it is in closing infinite debuggers. It makes a mockery of our tagline - "awesome debugging". And the extra irony is that I'm debugging some file path stuff for exercism, to make it easier for hopefully more people to learn Pharo.
How can we get this fixed?
Tim
On 2 Aug 2018, at 21:44, Norbert Hartl <norbert@hartl.name> wrote:
bump
Am 04.07.2018 um 02:28 schrieb Martin McClure <martin@hand2mouse.com>:
On 07/03/2018 05:02 PM, Martin McClure wrote:
On 06/29/2018 07:48 AM, Guillermo Polito wrote: I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Hi Guille, I'm just back from vacation last week, and about to go on vacation for another week, but I'll see what I can see. About the primitive pragmas for context-marking, I think some of those were changed for the exception handling fix that Andres and I did a few years back, so *could* be involved in this. I'd hate to see regression in the exception handling in an attempt to fix this bug.
After a look at at the pull request, I'm quite sure that removing the prim 199 marker is the wrong thing to do. Thanks, Pablo, for restoring it! The start of execution of exception handlers must be marked in order for #findNextHandlerContext to work correctly. If these contexts are not marked, exceptions signaled from inside an exception handler can find the wrong handler. See the code and comment in #findNextHandlerContext.
I'm afraid that I cannot immediately help with the debugger problem, since I don't know the debugger nearly as well as I do the exception handling code, and I'm going on vacation for a week in 20 minutes. :-) Perhaps when I get back I can take a look at it if it is still a problem by then.
Regards, -Martin
--
Guille Polito
Research Engineer
Centre de Recherche en Informatique, Signal et Automatique de Lille
CRIStAL - UMR 9189
French National Center for Scientific Research - *http://www.cnrs.fr <http://www.cnrs.fr>*
*Web:* *http://guillep.github.io* <http://guillep.github.io>
*Phone: *+33 06 52 70 66 13
-- Pablo Tesone. tesonep@gmail.com
This is a good writeup - however I get infinite debuggers when not running tests too? So Iâm wondering if there are multiple problems - or if this hints at a wider issue? Its not all of the time - but when you get it it, it seems to happen over and over again. Its possible its when using step-over though - so I will keep an eye on that. Tim
On 7 Aug 2018, at 10:51, tesonep@gmail.com wrote:
Hello, We have implemented a "patch" for Pharo 7, that is already integrated. I have created a slice to backport the "patch" to Pharo6.
Basically, the previous situation is the following:
- In the normal execution everything works, the problem is during the execution of over. - To implement over the debugger uses Context >> runUntilErrorOrReturnFrom: - This method adds a new context to handle exceptions in the stack trace. It takes the receiver and replaces the sender of the context with a new context handling the exceptions. - This error handling (using on:do:) handles UnhandledError and Halt. - When the MNU error gots to the nil context a UnhandledError is signal from the last signalling context of the MNU error. - This solution works correctly when it is not debugged when running a test.
When we are running a tests there is a slight difference:
- There is one more on:do: context that catches all the Errors. This on:do just do something and pass the exception. - As the UnhandledError is signal from the last signalling context (the one with the pass) it is not passing in the context inserted by runUntilErrorOrReturnFrom:, generating an infinite debugger as the exception is never catch. - This only happens with the MNU, as it retries to send the same message every time.
I think the solution is to signal the UnhandledError always from the original signalling context. However, I am not sure to perform that change as it modifies the behaviour of exceptions and there are not proper tests to guarantee the expected behavior.
I hope the problem is well explained, if it is not please ask me to clarify any point.
Cheers,
On Mon, Aug 6, 2018 at 9:59 AM Guillermo Polito <guillermopolito@gmail.com <mailto:guillermopolito@gmail.com>> wrote: Hi Steven,
The thing about your fix was mainly that it only worked for the case of running tests. We could however reproduce this bug from a playground too.
At first, replacing #pass to #debug looked like a hack to me :) Just looking at the code of runCaseForDebug:, I felt the correct thing to do there was to use #pass (and let any potential caller handle the exception if he wants). Otherwise, we should be able to understand: - can #pass be used? is it buggy? - does it work on any case or it should be avoided in some cases? - and how can we fix it (or at least document it?)?
But still, I think you were in the right direction too, because your fix shows a good intuition too: both #pass and #debug will do different things with the exception. - #pass will **resignal** it from the current context, - #debug will just open a debugger on it.
So that means that probably there was a problem when the exception was handled in a calling context.
On Mon, Aug 6, 2018 at 12:29 AM Steven Costiou <steven.costiou@kloum.io <mailto:steven.costiou@kloum.io>> wrote: Hi,
i had no answer to my comments on fogbuz (one of the first) so i assumed it was not a good idea.
In the usecases i used to reproduce the bug, replacing "ex pass" by "ex debug" in runCaseForDebug:solved the problem. See the analysis on fogbuz.
Did not have any side effect, but i do not know enough about the system and maybe it does. I think i already asked about that somewhere (here or on discord) but no answers.
But maybe it is wrong to do that? I'd be happy to have feedback on that.
Steven.
Le 2018-08-05 22:27, Tim Mackinnon a écrit :
Guys - this really needs attention - I've spend hours now trying to debug some code and most of it is in closing infinite debuggers. It makes a mockery of our tagline - "awesome debugging". And the extra irony is that I'm debugging some file path stuff for exercism, to make it easier for hopefully more people to learn Pharo.
How can we get this fixed?
Tim
On 2 Aug 2018, at 21:44, Norbert Hartl <norbert@hartl.name <mailto:norbert@hartl.name>> wrote:
bump
Am 04.07.2018 um 02:28 schrieb Martin McClure <martin@hand2mouse.com <mailto:martin@hand2mouse.com>>:
On 07/03/2018 05:02 PM, Martin McClure wrote:
On 06/29/2018 07:48 AM, Guillermo Polito wrote: I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Hi Guille, I'm just back from vacation last week, and about to go on vacation for another week, but I'll see what I can see. About the primitive pragmas for context-marking, I think some of those were changed for the exception handling fix that Andres and I did a few years back, so *could* be involved in this. I'd hate to see regression in the exception handling in an attempt to fix this bug.
After a look at at the pull request, I'm quite sure that removing the prim 199 marker is the wrong thing to do. Thanks, Pablo, for restoring it! The start of execution of exception handlers must be marked in order for #findNextHandlerContext to work correctly. If these contexts are not marked, exceptions signaled from inside an exception handler can find the wrong handler. See the code and comment in #findNextHandlerContext.
I'm afraid that I cannot immediately help with the debugger problem, since I don't know the debugger nearly as well as I do the exception handling code, and I'm going on vacation for a week in 20 minutes. :-) Perhaps when I get back I can take a look at it if it is still a problem by then.
Regards, -Martin
--
Guille Polito Research Engineer
Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - http://www.cnrs.fr <http://www.cnrs.fr/>
Web: http://guillep.github.io <http://guillep.github.io/> Phone: +33 06 52 70 66 13
-- Pablo Tesone. tesonep@gmail.com <mailto:tesonep@gmail.com>
Also, we have found that once the problem starts the image will present bad behavior from that on. Sometimes it can be fixed by restarting the UIProcess, but it is a brute force process. You have to do it many times. It is true that we cannot assert that the bug is not in other places. On Tue, Aug 7, 2018 at 11:14 AM Tim Mackinnon <tim@testit.works> wrote:
This is a good writeup - however I get infinite debuggers when not running tests too? So Iâm wondering if there are multiple problems - or if this hints at a wider issue?
Its not all of the time - but when you get it it, it seems to happen over and over again. Its possible its when using step-over though - so I will keep an eye on that.
Tim
On 7 Aug 2018, at 10:51, tesonep@gmail.com wrote:
Hello, We have implemented a "patch" for Pharo 7, that is already integrated. I have created a slice to backport the "patch" to Pharo6.
Basically, the previous situation is the following:
- In the normal execution everything works, the problem is during the execution of over. - To implement over the debugger uses Context >> runUntilErrorOrReturnFrom: - This method adds a new context to handle exceptions in the stack trace. It takes the receiver and replaces the sender of the context with a new context handling the exceptions. - This error handling (using on:do:) handles UnhandledError and Halt. - When the MNU error gots to the nil context a UnhandledError is signal from the last signalling context of the MNU error. - This solution works correctly when it is not debugged when running a test.
When we are running a tests there is a slight difference:
- There is one more on:do: context that catches all the Errors. This on:do just do something and pass the exception. - As the UnhandledError is signal from the last signalling context (the one with the pass) it is not passing in the context inserted by runUntilErrorOrReturnFrom:, generating an infinite debugger as the exception is never catch. - This only happens with the MNU, as it retries to send the same message every time.
I think the solution is to signal the UnhandledError always from the original signalling context. However, I am not sure to perform that change as it modifies the behaviour of exceptions and there are not proper tests to guarantee the expected behavior.
I hope the problem is well explained, if it is not please ask me to clarify any point.
Cheers,
On Mon, Aug 6, 2018 at 9:59 AM Guillermo Polito <guillermopolito@gmail.com> wrote:
Hi Steven,
The thing about your fix was mainly that it only worked for the case of running tests. We could however reproduce this bug from a playground too.
At first, replacing #pass to #debug looked like a hack to me :) Just looking at the code of runCaseForDebug:, I felt the correct thing to do there was to use #pass (and let any potential caller handle the exception if he wants). Otherwise, we should be able to understand: - can #pass be used? is it buggy? - does it work on any case or it should be avoided in some cases? - and how can we fix it (or at least document it?)?
But still, I think you were in the right direction too, because your fix shows a good intuition too: both #pass and #debug will do different things with the exception. - #pass will **resignal** it from the current context, - #debug will just open a debugger on it.
So that means that probably there was a problem when the exception was handled in a calling context.
On Mon, Aug 6, 2018 at 12:29 AM Steven Costiou <steven.costiou@kloum.io> wrote:
Hi,
i had no answer to my comments on fogbuz (one of the first) so i assumed it was not a good idea.
In the usecases i used to reproduce the bug, replacing "ex pass" by "ex debug" in runCaseForDebug:solved the problem. See the analysis on fogbuz.
Did not have any side effect, but i do not know enough about the system and maybe it does. I think i already asked about that somewhere (here or on discord) but no answers.
But maybe it is wrong to do that? I'd be happy to have feedback on that.
Steven.
Le 2018-08-05 22:27, Tim Mackinnon a écrit :
Guys - this really needs attention - I've spend hours now trying to debug some code and most of it is in closing infinite debuggers. It makes a mockery of our tagline - "awesome debugging". And the extra irony is that I'm debugging some file path stuff for exercism, to make it easier for hopefully more people to learn Pharo.
How can we get this fixed?
Tim
On 2 Aug 2018, at 21:44, Norbert Hartl <norbert@hartl.name> wrote:
bump
Am 04.07.2018 um 02:28 schrieb Martin McClure <martin@hand2mouse.com>:
On 07/03/2018 05:02 PM, Martin McClure wrote:
On 06/29/2018 07:48 AM, Guillermo Polito wrote: I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Hi Guille, I'm just back from vacation last week, and about to go on vacation for another week, but I'll see what I can see. About the primitive pragmas for context-marking, I think some of those were changed for the exception handling fix that Andres and I did a few years back, so *could* be involved in this. I'd hate to see regression in the exception handling in an attempt to fix this bug.
After a look at at the pull request, I'm quite sure that removing the prim 199 marker is the wrong thing to do. Thanks, Pablo, for restoring it! The start of execution of exception handlers must be marked in order for #findNextHandlerContext to work correctly. If these contexts are not marked, exceptions signaled from inside an exception handler can find the wrong handler. See the code and comment in #findNextHandlerContext.
I'm afraid that I cannot immediately help with the debugger problem, since I don't know the debugger nearly as well as I do the exception handling code, and I'm going on vacation for a week in 20 minutes. :-) Perhaps when I get back I can take a look at it if it is still a problem by then.
Regards, -Martin
--
Guille Polito Research Engineer
Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - *http://www.cnrs.fr <http://www.cnrs.fr/>*
*Web:* *http://guillep.github.io* <http://guillep.github.io/> *Phone: *+33 06 52 70 66 13
-- Pablo Tesone. tesonep@gmail.com
-- Pablo Tesone. tesonep@gmail.com
El dom., 5 de ago. de 2018 17:28, Tim Mackinnon <tim@testit.works> escribió:
Guys - this really needs attention - Iâve spend hours now trying to debug some code and most of it is in closing infinite debuggers. It makes a mockery of our tagline - âawesome debuggingâ.
We can change the tagline to "limitless debugging".
Hi tim We know and we made huge progress because before we could not even reproduce it. We spent some times on it. I thought the solution found by pablo and santiago got integrated. Stef
Guys - this really needs attention - Iâve spend hours now trying to debug some code and most of it is in closing infinite debuggers. It makes a mockery of our tagline - âawesome debuggingâ. And the extra irony is that Iâm debugging some file path stuff for exercism, to make it easier for hopefully more people to learn Pharo.
How can we get this fixed?
Tim
On 2 Aug 2018, at 21:44, Norbert Hartl <norbert@hartl.name> wrote:
bump
Am 04.07.2018 um 02:28 schrieb Martin McClure <martin@hand2mouse.com>:
On 07/03/2018 05:02 PM, Martin McClure wrote:
On 06/29/2018 07:48 AM, Guillermo Polito wrote: I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Hi Guille, I'm just back from vacation last week, and about to go on vacation for another week, but I'll see what I can see. About the primitive pragmas for context-marking, I think some of those were changed for the exception handling fix that Andres and I did a few years back, so *could* be involved in this. I'd hate to see regression in the exception handling in an attempt to fix this bug.
After a look at at the pull request, I'm quite sure that removing the prim 199 marker is the wrong thing to do. Thanks, Pablo, for restoring it! The start of execution of exception handlers must be marked in order for #findNextHandlerContext to work correctly. If these contexts are not marked, exceptions signaled from inside an exception handler can find the wrong handler. See the code and comment in #findNextHandlerContext.
I'm afraid that I cannot immediately help with the debugger problem, since I don't know the debugger nearly as well as I do the exception handling code, and I'm going on vacation for a week in 20 minutes. :-) Perhaps when I get back I can take a look at it if it is still a problem by then.
Regards, -Martin
-------------------------------------------- Stéphane Ducasse http://stephane.ducasse.free.fr http://www.synectique.eu / http://www.pharo.org 03 59 35 87 52 Assistant: Julie Jonas FAX 03 59 57 78 50 TEL 03 59 35 86 16 S. Ducasse - Inria 40, avenue Halley, Parc Scientifique de la Haute Borne, Bât.A, Park Plaza Villeneuve d'Ascq 59650 France
Hi Tim, Are you on Pharo 6 or 7? If in 7, what build are you using? Pablo and Santi have made a fix 2 fridays ago (that I presume got integrated last week). The fix consists on changing a bit the exception handling during exception handling. https://github.com/pharo-project/pharo/pull/1621/files The fix seems simple, but it has a lot of information contained on it (like the fact that the system introduces exception handlers in the middle of the stack transparently to manage errors while stepping, or the fact that UnhandledError does another traversal of the stack but needs to start at the good place...). The Exception tests we had are still green, and we can now do stepping on code that raises exceptions. When stepping inside the DNU multiple times the debugging experience is not as smooth as the one in Pharo 3 (before it got broken) but this is FAR BETTER than Pharo7 since we have not experienced new infinite debuggers anymore :)... I'll let Pablo and Santi answer more properly with the technical details. On my side, I think we need to better document and do some more unit tests on that dark part of the system :). On Mon, Aug 6, 2018 at 8:50 AM Stéphane Ducasse <stephane.ducasse@inria.fr> wrote:
Hi tim
We know and we made huge progress because before we could not even reproduce it. We spent some times on it. I thought the solution found by pablo and santiago got integrated.
Stef
Guys - this really needs attention - Iâve spend hours now trying to debug some code and most of it is in closing infinite debuggers. It makes a mockery of our tagline - âawesome debuggingâ. And the extra irony is that Iâm debugging some file path stuff for exercism, to make it easier for hopefully more people to learn Pharo.
How can we get this fixed?
Tim
On 2 Aug 2018, at 21:44, Norbert Hartl <norbert@hartl.name> wrote:
bump
Am 04.07.2018 um 02:28 schrieb Martin McClure <martin@hand2mouse.com>:
On 07/03/2018 05:02 PM, Martin McClure wrote:
On 06/29/2018 07:48 AM, Guillermo Polito wrote: I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Hi Guille, I'm just back from vacation last week, and about to go on vacation for another week, but I'll see what I can see. About the primitive pragmas for context-marking, I think some of those were changed for the exception handling fix that Andres and I did a few years back, so *could* be involved in this. I'd hate to see regression in the exception handling in an attempt to fix this bug.
After a look at at the pull request, I'm quite sure that removing the prim 199 marker is the wrong thing to do. Thanks, Pablo, for restoring it! The start of execution of exception handlers must be marked in order for #findNextHandlerContext to work correctly. If these contexts are not marked, exceptions signaled from inside an exception handler can find the wrong handler. See the code and comment in #findNextHandlerContext.
I'm afraid that I cannot immediately help with the debugger problem, since I don't know the debugger nearly as well as I do the exception handling code, and I'm going on vacation for a week in 20 minutes. :-) Perhaps when I get back I can take a look at it if it is still a problem by then.
Regards, -Martin
-------------------------------------------- Stéphane Ducasse http://stephane.ducasse.free.fr http://www.synectique.eu / http://www.pharo.org 03 59 35 87 52 Assistant: Julie Jonas FAX 03 59 57 78 50 TEL 03 59 35 86 16 S. Ducasse - Inria 40, avenue Halley, Parc Scientifique de la Haute Borne, Bât.A, Park Plaza Villeneuve d'Ascq 59650 France
-- Guille Polito Research Engineer Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - *http://www.cnrs.fr <http://www.cnrs.fr>* *Web:* *http://guillep.github.io* <http://guillep.github.io> *Phone: *+33 06 52 70 66 13
Hey thanks - this is Pharo 6.1, with a zero conf downloaded yesterday, and a second image from a week ago. I guess its possible that I donât have those changes - is there an easy way to check? Iâm happy to try applying them - or use a different image to test with - as it seems that certain conditions really rear their head and then you just get the problem over and over. On the plus side - its rare that you crash you image and then have to recover changes - but its just annoying when it gets in the way of debugging. ITs not just all the windows, its also the fact that none of the debugger windows actually puts you in a useful stack where you can see the problem - they are all stuck on DNU with a single line stack. Tim
On 6 Aug 2018, at 09:46, Guillermo Polito <guillermopolito@gmail.com> wrote:
Hi Tim,
Are you on Pharo 6 or 7? If in 7, what build are you using?
Pablo and Santi have made a fix 2 fridays ago (that I presume got integrated last week). The fix consists on changing a bit the exception handling during exception handling.
https://github.com/pharo-project/pharo/pull/1621/files <https://github.com/pharo-project/pharo/pull/1621/files>
The fix seems simple, but it has a lot of information contained on it (like the fact that the system introduces exception handlers in the middle of the stack transparently to manage errors while stepping, or the fact that UnhandledError does another traversal of the stack but needs to start at the good place...). The Exception tests we had are still green, and we can now do stepping on code that raises exceptions. When stepping inside the DNU multiple times the debugging experience is not as smooth as the one in Pharo 3 (before it got broken) but this is FAR BETTER than Pharo7 since we have not experienced new infinite debuggers anymore :)...
I'll let Pablo and Santi answer more properly with the technical details.
On my side, I think we need to better document and do some more unit tests on that dark part of the system :).
On Mon, Aug 6, 2018 at 8:50 AM Stéphane Ducasse <stephane.ducasse@inria.fr <mailto:stephane.ducasse@inria.fr>> wrote: Hi tim
We know and we made huge progress because before we could not even reproduce it. We spent some times on it. I thought the solution found by pablo and santiago got integrated.
Stef
Guys - this really needs attention - Iâve spend hours now trying to debug some code and most of it is in closing infinite debuggers. It makes a mockery of our tagline - âawesome debuggingâ. And the extra irony is that Iâm debugging some file path stuff for exercism, to make it easier for hopefully more people to learn Pharo.
How can we get this fixed?
Tim
On 2 Aug 2018, at 21:44, Norbert Hartl <norbert@hartl.name <mailto:norbert@hartl.name>> wrote:
bump
Am 04.07.2018 um 02:28 schrieb Martin McClure <martin@hand2mouse.com <mailto:martin@hand2mouse.com>>:
On 07/03/2018 05:02 PM, Martin McClure wrote:
On 06/29/2018 07:48 AM, Guillermo Polito wrote: I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Hi Guille, I'm just back from vacation last week, and about to go on vacation for another week, but I'll see what I can see. About the primitive pragmas for context-marking, I think some of those were changed for the exception handling fix that Andres and I did a few years back, so *could* be involved in this. I'd hate to see regression in the exception handling in an attempt to fix this bug.
After a look at at the pull request, I'm quite sure that removing the prim 199 marker is the wrong thing to do. Thanks, Pablo, for restoring it! The start of execution of exception handlers must be marked in order for #findNextHandlerContext to work correctly. If these contexts are not marked, exceptions signaled from inside an exception handler can find the wrong handler. See the code and comment in #findNextHandlerContext.
I'm afraid that I cannot immediately help with the debugger problem, since I don't know the debugger nearly as well as I do the exception handling code, and I'm going on vacation for a week in 20 minutes. :-) Perhaps when I get back I can take a look at it if it is still a problem by then.
Regards, -Martin
-------------------------------------------- Stéphane Ducasse http://stephane.ducasse.free.fr <http://stephane.ducasse.free.fr/> http://www.synectique.eu <http://www.synectique.eu/> / http://www.pharo.org <http://www.pharo.org/> 03 59 35 87 52 Assistant: Julie Jonas FAX 03 59 57 78 50 TEL 03 59 35 86 16 S. Ducasse - Inria 40, avenue Halley, Parc Scientifique de la Haute Borne, Bât.A, Park Plaza Villeneuve d'Ascq 59650 France
--
Guille Polito Research Engineer
Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - http://www.cnrs.fr <http://www.cnrs.fr/>
Web: http://guillep.github.io <http://guillep.github.io/> Phone: +33 06 52 70 66 13
On 6 Aug 2018, at 09:54, Tim Mackinnon <tim@testit.works> wrote:
Hey thanks - this is Pharo 6.1, with a zero conf downloaded yesterday, and a second image from a week ago. I guess its possible that I donât have those changes - is there an easy way to check?
These changes are not in Pharo 6.1 but they should because this is terrible! And a new version of the mooc is coming.
Iâm happy to try applying them - or use a different image to test with - as it seems that certain conditions really rear their head and then you just get the problem over and over.
On the plus side - its rare that you crash you image and then have to recover changes - but its just annoying when it gets in the way of debugging.
No it is SUPER SUPER ANNOYING.
ITs not just all the windows, its also the fact that none of the debugger windows actually puts you in a useful stack where you can see the problem - they are all stuck on DNU with a single line stack.
Tim
On 6 Aug 2018, at 09:46, Guillermo Polito <guillermopolito@gmail.com <mailto:guillermopolito@gmail.com>> wrote:
Hi Tim,
Are you on Pharo 6 or 7? If in 7, what build are you using?
Pablo and Santi have made a fix 2 fridays ago (that I presume got integrated last week). The fix consists on changing a bit the exception handling during exception handling.
https://github.com/pharo-project/pharo/pull/1621/files <https://github.com/pharo-project/pharo/pull/1621/files>
The fix seems simple, but it has a lot of information contained on it (like the fact that the system introduces exception handlers in the middle of the stack transparently to manage errors while stepping, or the fact that UnhandledError does another traversal of the stack but needs to start at the good place...). The Exception tests we had are still green, and we can now do stepping on code that raises exceptions. When stepping inside the DNU multiple times the debugging experience is not as smooth as the one in Pharo 3 (before it got broken) but this is FAR BETTER than Pharo7 since we have not experienced new infinite debuggers anymore :)...
I'll let Pablo and Santi answer more properly with the technical details.
On my side, I think we need to better document and do some more unit tests on that dark part of the system :).
On Mon, Aug 6, 2018 at 8:50 AM Stéphane Ducasse <stephane.ducasse@inria.fr <mailto:stephane.ducasse@inria.fr>> wrote: Hi tim
We know and we made huge progress because before we could not even reproduce it. We spent some times on it. I thought the solution found by pablo and santiago got integrated.
Stef
Guys - this really needs attention - Iâve spend hours now trying to debug some code and most of it is in closing infinite debuggers. It makes a mockery of our tagline - âawesome debuggingâ. And the extra irony is that Iâm debugging some file path stuff for exercism, to make it easier for hopefully more people to learn Pharo.
How can we get this fixed?
Tim
On 2 Aug 2018, at 21:44, Norbert Hartl <norbert@hartl.name <mailto:norbert@hartl.name>> wrote:
bump
Am 04.07.2018 um 02:28 schrieb Martin McClure <martin@hand2mouse.com <mailto:martin@hand2mouse.com>>:
On 07/03/2018 05:02 PM, Martin McClure wrote:
On 06/29/2018 07:48 AM, Guillermo Polito wrote: I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Hi Guille, I'm just back from vacation last week, and about to go on vacation for another week, but I'll see what I can see. About the primitive pragmas for context-marking, I think some of those were changed for the exception handling fix that Andres and I did a few years back, so *could* be involved in this. I'd hate to see regression in the exception handling in an attempt to fix this bug.
After a look at at the pull request, I'm quite sure that removing the prim 199 marker is the wrong thing to do. Thanks, Pablo, for restoring it! The start of execution of exception handlers must be marked in order for #findNextHandlerContext to work correctly. If these contexts are not marked, exceptions signaled from inside an exception handler can find the wrong handler. See the code and comment in #findNextHandlerContext.
I'm afraid that I cannot immediately help with the debugger problem, since I don't know the debugger nearly as well as I do the exception handling code, and I'm going on vacation for a week in 20 minutes. :-) Perhaps when I get back I can take a look at it if it is still a problem by then.
Regards, -Martin
-------------------------------------------- Stéphane Ducasse http://stephane.ducasse.free.fr <http://stephane.ducasse.free.fr/> http://www.synectique.eu <http://www.synectique.eu/> / http://www.pharo.org <http://www.pharo.org/> 03 59 35 87 52 Assistant: Julie Jonas FAX 03 59 57 78 50 TEL 03 59 35 86 16 S. Ducasse - Inria 40, avenue Halley, Parc Scientifique de la Haute Borne, Bât.A, Park Plaza Villeneuve d'Ascq 59650 France
--
Guille Polito Research Engineer
Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - http://www.cnrs.fr <http://www.cnrs.fr/>
Web: http://guillep.github.io <http://guillep.github.io/> Phone: +33 06 52 70 66 13
-------------------------------------------- Stéphane Ducasse http://stephane.ducasse.free.fr http://www.synectique.eu / http://www.pharo.org 03 59 35 87 52 Assistant: Julie Jonas FAX 03 59 57 78 50 TEL 03 59 35 86 16 S. Ducasse - Inria 40, avenue Halley, Parc Scientifique de la Haute Borne, Bât.A, Park Plaza Villeneuve d'Ascq 59650 France
Hi Tim, Just in the event you didn't know there is an option in the World menu to close all the open debuggers. Its in the "Windows" section, about half way down. Paul Tim Mackinnon wrote
...
On the plus side - its rare that you crash you image and then have to recover changes - but its just annoying when it gets in the way of debugging. ITs not just all the windows, its also the fact that none of the debugger windows actually puts you in a useful stack where you can see the problem - they are all stuck on DNU with a single line stack.
Tim
...
-- Sent from: http://forum.world.st/Pharo-Smalltalk-Developers-f1294837.html
Thanks Paul - Iâd forgotten about that option (been using the close all to right option). Itâs just jarring - but worse is that none of the debuggers (even the first one) having anything useful in them... other than the error msg in the title. So you have to start all over again. Still, it sounds like we are honing in on something. Tim Sent from my iPhone
On 7 Aug 2018, at 19:38, Paul DeBruicker <pdebruic@gmail.com> wrote:
Hi Tim,
Just in the event you didn't know there is an option in the World menu to close all the open debuggers.
Its in the "Windows" section, about half way down.
Paul
Tim Mackinnon wrote
...
On the plus side - its rare that you crash you image and then have to recover changes - but its just annoying when it gets in the way of debugging. ITs not just all the windows, its also the fact that none of the debugger windows actually puts you in a useful stack where you can see the problem - they are all stuck on DNU with a single line stack.
Tim
...
-- Sent from: http://forum.world.st/Pharo-Smalltalk-Developers-f1294837.html
On 08/06/2018 12:46 AM, Guillermo Polito wrote:
Pablo and Santi have made a fix 2 fridays ago (that I presume got integrated last week). The fix consists on changing a bit the exception handling during exception handling.
https://github.com/pharo-project/pharo/pull/1621/files
The fix seems simple, but it has a lot of information contained on it (like the fact that the system introduces exception handlers in the middle of the stack transparently to manage errors while stepping, or the fact that UnhandledError does another traversal of the stack but needs to start at the good place...). The Exception tests we had are still green, and we can now do stepping on code that raises exceptions. When stepping inside the DNU multiple times the debugging experience is not as smooth as the one in Pharo 3 (before it got broken) but this is FAR BETTER than Pharo7 since we have not experienced new infinite debuggers anymore :)...
Thanks for the update, Guille, and the pointer to the fix. This fix smells a bit hacky, but... I don't know the internals of the debugger. It might be the best way to handle it. And I'm glad it has improved the behavior. Since I *am* familiar with the changes to exception handling, and the reasons behind those changes, I'd be happy to sit down at Camp Smalltalk in Cagliari with you or whoever knows a bit about this problem, and see whether it's possible to make a smoother fix. Regards, -Martin
Hi. Good job guys. I just checked my case 19848 <https://pharo.fogbugz.com/f/cases/19848/Step-over-or-though-expression-with-...>. And it is now fixed. Thanks 2018-06-29 15:48 GMT+01:00 Guillermo Polito <guillermopolito@gmail.com>:
Hi all,
during today's sprint we have been working with lots of people on the infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We have checked the emails sent in the latest month. Then, together with Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis all day. We have been also comparing the debuggers code between pharo 3/4 (where the bug was is present) and pharo 7, but this was not necessarily straight forward as the code is not the same and there is no easy diff...
AAAAND, we have found that the problem may come from a wrong pragma marker. Just removing that pragma gives us back the same behaviour as in Pharo 3/4. :D
https://github.com/pharo-project/pharo/pull/1621
I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Thanks all for the fish, Guille
I have more special cases where debugger hangs: 19662 <https://pharo.fogbugz.com/f/cases/19662/Debugger-step-through-mustBeBoolean-...> , 19928 <https://pharo.fogbugz.com/f/cases/19928/Debugger-step-over-through-last-doIt...> , 20274 <https://pharo.fogbugz.com/f/cases/20274/Debugger-infinite-recursion-when-ter...> . They are still relevant 2018-08-07 22:01 GMT+01:00 Denis Kudriashov <dionisiydk@gmail.com>:
Hi.
Good job guys. I just checked my case 19848 <https://pharo.fogbugz.com/f/cases/19848/Step-over-or-though-expression-with-...>. And it is now fixed.
Thanks
2018-06-29 15:48 GMT+01:00 Guillermo Polito <guillermopolito@gmail.com>:
Hi all,
during today's sprint we have been working with lots of people on the infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We have checked the emails sent in the latest month. Then, together with Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis all day. We have been also comparing the debuggers code between pharo 3/4 (where the bug was is present) and pharo 7, but this was not necessarily straight forward as the code is not the same and there is no easy diff...
AAAAND, we have found that the problem may come from a wrong pragma marker. Just removing that pragma gives us back the same behaviour as in Pharo 3/4. :D
https://github.com/pharo-project/pharo/pull/1621
I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Thanks all for the fish, Guille
Yes, we have to check all the scenarios. The problems with the UI process is not resolved at all. On Tue, Aug 7, 2018 at 11:11 PM Denis Kudriashov <dionisiydk@gmail.com> wrote:
I have more special cases where debugger hangs: 19662 <https://pharo.fogbugz.com/f/cases/19662/Debugger-step-through-mustBeBoolean-...> , 19928 <https://pharo.fogbugz.com/f/cases/19928/Debugger-step-over-through-last-doIt...> , 20274 <https://pharo.fogbugz.com/f/cases/20274/Debugger-infinite-recursion-when-ter...> . They are still relevant
2018-08-07 22:01 GMT+01:00 Denis Kudriashov <dionisiydk@gmail.com>:
Hi.
Good job guys. I just checked my case 19848 <https://pharo.fogbugz.com/f/cases/19848/Step-over-or-though-expression-with-...>. And it is now fixed.
Thanks
2018-06-29 15:48 GMT+01:00 Guillermo Polito <guillermopolito@gmail.com>:
Hi all,
during today's sprint we have been working with lots of people on the infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We have checked the emails sent in the latest month. Then, together with Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis all day. We have been also comparing the debuggers code between pharo 3/4 (where the bug was is present) and pharo 7, but this was not necessarily straight forward as the code is not the same and there is no easy diff...
AAAAND, we have found that the problem may come from a wrong pragma marker. Just removing that pragma gives us back the same behaviour as in Pharo 3/4. :D
https://github.com/pharo-project/pharo/pull/1621
I know that the exception handling/debugging has been modified several times in the latest years (some refactorings, hiding contexts...), we unfortunately don't have tests for it, so I'd like some more pair of eyes on it. Ben, Martin could you take a look?
Thanks all for the fish, Guille
-- Pablo Tesone. tesonep@gmail.com
participants (14)
-
Ben Coman -
Clément Bera -
Denis Kudriashov -
Eliot Miranda -
Esteban A. Maringolo -
Guillermo Polito -
Marcus Denker -
Martin McClure -
Norbert Hartl -
Paul DeBruicker -
Steven Costiou -
Stéphane Ducasse -
tesonep＠gmail.com -
Tim Mackinnon