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

Do not raise when an instance already exists (fixes issue #607) #610

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

souliane
Copy link

@souliane souliane commented Aug 30, 2023

Do not raise when an instance already exists (fixes issue #607)

I assume that this is a dirty fix, yet it works for me.
Please let me know what is the good way to fix the issue, I would like to tackle it.
How to reproduce is described here.

Comment on lines -189 to -190
if Debugger._current_debugger:
raise ValueError("a Debugger instance already exists")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with this is that various parts of the code assume that only one of these objects ever exists, e.g. here:

pudb/pudb/__init__.py

Lines 74 to 90 in b9c1948

def _get_debugger(**kwargs):
from pudb.debugger import Debugger
if not Debugger._current_debugger:
tty_path = _tty_override()
if tty_path and ("stdin" not in kwargs or "stdout" not in kwargs):
tty_file, term_size = _open_tty(tty_path)
kwargs.setdefault("stdin", tty_file)
kwargs.setdefault("stdout", tty_file)
kwargs.setdefault("term_size", term_size)
tty_file.close()
from pudb.debugger import Debugger
dbg = Debugger(**kwargs)
return dbg
else:
return Debugger._current_debugger[0]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see... can you think of way to fix the issue without messing with those parts of the code? Maybe we can destroy the pudb instance when c is pressed?

@asmeurer
Copy link
Collaborator

This seems like the wrong fix. If the debugger is already running then further breakpoints should just break the debugger. If that doesn't happen we should try to investigate why. Having multiple debugger classes implies recursive debugging, which sounds really complicated and probably not worth supporting.

@hwalinga
Copy link

@asmeurer Hello

I did an analysis of this. If you use pytest --pdbcls pudb.debugger:Debugger --pdb --capture=no test.py, pytest instantiates a new Debugger class (1) whenever a new breakpoint is encountered (!! with PYTHONBREAKPOINT not set !!) and (2) on post-mortem analysis.

Making Debugger a singleton using metaclasses should solve this. HOWEVER, an old bug from #52 resurfaced then. I tried to fix this in #667

What is more, pytest-pudb also has exhibits the bug in #52 when using below code:

def test_foo():
    breakpoint()
    raise Exception

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants