Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Behavior conditional on debugger is dangerous #11

Closed
haarg opened this issue May 24, 2022 · 6 comments
Closed

Behavior conditional on debugger is dangerous #11

haarg opened this issue May 24, 2022 · 6 comments
Labels
Discussion An item of concern that should be dsicussed

Comments

@haarg
Copy link
Contributor

haarg commented May 24, 2022

I think having behavior change depending on if the debugger is in use is a terrible idea. It is a recipe for heisenbugs and confusion.

Being immutable can have an impact on many things that are not obvious, like when other modules play around with @ISA or modify your symbol table. The warning issued could also produce a tremendous amount of spam in a larger code base.

Making namespace::autoclean conditional on the debugger is, I think, an even larger mistake. The extra subs will impact method resolution and leaving them in place could easily break classes.

Both of these are exactly the type of thing you might want to explore in the debugger.

$^P also can be used in cases other than using a full debugger. Devel::Confess, for example, enables only the flags to give more descriptive names to anonymous subs and evals. When using a module like Devel::Confess that uses a limited set of debug flags, there would not be any benefit at all to disabling make_immutable and namespace::autoclean.

@Ovid
Copy link
Owner

Ovid commented May 24, 2022

I could remove the namespace::autoclean bit, but if I don't disable after_runtime, the code dies and you can't even use the debugger.

If there was another (safer) way to apply make_immutable, that would possibly make this issue go away.

@Ovid Ovid added question Further information is requested Discussion An item of concern that should be dsicussed and removed question Further information is requested labels May 24, 2022
@Ovid
Copy link
Owner

Ovid commented May 24, 2022

namespace::autoclean is no longer removed when running under the debugger.

@Ovid
Copy link
Owner

Ovid commented May 31, 2022

Closing this as I can see no solution to the "make_immutable" issue under the debugger :( Feel free to reopen if you have any better ideas.

@Ovid Ovid closed this as completed May 31, 2022
@haarg
Copy link
Contributor Author

haarg commented Jun 3, 2022

The issue with after_runtime isn't actually due to it being run too late, it's just a bug in B::Hooks::AtRuntime.

I've filed a PR to fix it: mauzo/B-Hooks-AtRuntime#1

@Ovid
Copy link
Owner

Ovid commented Jun 3, 2022

Thank you! I'll reopen this ticket.

@Ovid Ovid reopened this Jun 3, 2022
@Ovid
Copy link
Owner

Ovid commented Jun 20, 2022

I've upgraded to the latest B::Hooks::AtRuntime and tested it under the debugger. Works great. This change is now in main. Thanks for the help.

@Ovid Ovid closed this as completed Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion An item of concern that should be dsicussed
Projects
None yet
Development

No branches or pull requests

2 participants