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

Crashes on ending debug session when using PyPy #1198

Closed
longnguyen2004 opened this issue Jan 31, 2023 · 18 comments
Closed

Crashes on ending debug session when using PyPy #1198

longnguyen2004 opened this issue Jan 31, 2023 · 18 comments
Labels
bug Something isn't working needs investigation P2

Comments

@longnguyen2004
Copy link

Before creating a new issue, please check the FAQ to see if your question is answered there.

Environment data

  • debugpy version: 1.6.5
  • Python version (& distribution if applicable, e.g. Anaconda): PyPy 7.3.11 (Python 3.9.16)
  • Using VS Code or Visual Studio: VS Code 1.74.3

Actual behavior

Program crashes on ending debug session with the following error

Exception in thread Adapter message handler:
Traceback (most recent call last):
  File "C:\pypy3.9-v7.3.11-win64\Lib\threading.py", line 980, in _bootstrap_inner
    self.run()
  File "C:\pypy3.9-v7.3.11-win64\Lib\threading.py", line 917, in run
    self._target(*self._args, **self._kwargs)
  File "c:\Users\Acer\.vscode\extensions\ms-python.python-2022.20.2\pythonFiles\lib\python\debugpy\launcher/../..\debugpy\common\messaging.py", line 1427, in _run_handlers
    self._parser_thread.join()
  File "C:\pypy3.9-v7.3.11-win64\Lib\threading.py", line 1060, in join
    self._wait_for_tstate_lock()
  File "C:\pypy3.9-v7.3.11-win64\Lib\threading.py", line 1081, in _wait_for_tstate_lock
    lock.release()
RuntimeError: cannot release un-acquired lock

Expected behavior

No crashes

Steps to reproduce:

  1. Download PyPy 7.3.11
  2. Debug a simple Hello World program
  3. See crash
@int19h int19h added the bug Something isn't working label Jan 31, 2023
@judej
Copy link

judej commented Jan 3, 2024

Closing old issue. If this is still a problem, please reopen with the information requested. thanks

@RedHeadphone
Copy link

RedHeadphone commented Sep 1, 2024

@judej

This is still an issue (not sure if it's at pypy end or debugpy)
using PyPy 7.3.17 (Python 3.10.14) as python interpretor with debugpy to debug in vscode, the above error is thrown after the execution.

@TechPro424
Copy link

Requesting for this issue to be reopened

  • debugpy version: 1.8.2
  • Python version (& distribution if applicable, e.g. Anaconda): PyPy 7.3.17 (Python 3.10.14)
  • Using VS Code or Visual Studio: VS Code 1.93.1

Program crashes on ending debug session with the following error

Exception in thread Adapter message handler:
Traceback (most recent call last):
  File "C:\Users\TechPro424\scoop\apps\pypy3\current\Lib\threading.py", line 1016, in _bootstrap_inner
    self.run()
  File "C:\Users\TechPro424\scoop\apps\pypy3\current\Lib\threading.py", line 953, in run
    self._target(*self._args, **self._kwargs)
  File "C:\Users\TechPro424\scoop\persist\vscodium\data\extensions\ms-python.debugpy-2024.11.0-dev-win32-x64\bundled\libs\debugpy\launcher/../..\debugpy\common\messaging.py", line 1427, in _run_handlers
    self._parser_thread.join()
  File "C:\Users\TechPro424\scoop\apps\pypy3\current\Lib\threading.py", line 1096, in join
    self._wait_for_tstate_lock()
  File "C:\Users\TechPro424\scoop\apps\pypy3\current\Lib\threading.py", line 1117, in _wait_for_tstate_lock
    lock.release()
RuntimeError: cannot release un-acquired lock

Expected behavior: No crashes

Steps to reproduce:

  • Download PyPy 7.3.17
  • Debug the following program
L2=[10,20,30, 40, 50]
L2.reverse()
print(L2)
  • See crash

@rchiodo
Copy link
Contributor

rchiodo commented Sep 23, 2024

This seems like a bug in PyPy to me. Debugpy is calling thread.join. PyPy's implementation of thread.join cannot handle the join.

@mattip
Copy link

mattip commented Sep 25, 2024

Does the crash always happen? It seems very strange. The code, which is taken directly from CPython calls lock.aquire(...) and if it succeeds calls lock.release():

            if lock.acquire(block, timeout):
                lock.release()

But I also see that debugpy is switching out a lock, maybe that is somehow messing things up?

@mattip
Copy link

mattip commented Sep 25, 2024

Note the original thread lock code uses _thread._set_sentinel() not _thread.allocate_lock(). @TechPro424 if you modify your installation of debugpy to create a sentinel lock does that fix the problem? The code is in pydevd/pydevd_attach_to_process/attach_script.py line 112

@TechPro424
Copy link

change threading._allocate_lock() to threading._set_sentinel() right?
Yeah that works

@mattip
Copy link

mattip commented Sep 26, 2024

Cool. @judej, @rchiodo could debugpy change the lock? I would submit the change but it requires a CLA and all that, which is too many hoops for a one line change.

diff --git a/src/debugpy/_vendored/pydevd/pydevd_attach_to_process/attach_script.py b/src/debugpy/_vendored/pydevd/pydevd_attach_to_process/attach_script.py
index eb5f8d0a..78d397f5 100644
--- a/src/debugpy/_vendored/pydevd/pydevd_attach_to_process/attach_script.py
+++ b/src/debugpy/_vendored/pydevd/pydevd_attach_to_process/attach_script.py
@@ -109,7 +109,7 @@ def fix_main_thread_id(on_warn=lambda msg: None, on_exception=lambda msg: None,
                     # to the thread state and will be released when the secondary thread
                     # that initialized the lock is finished -- making an assert fail during
                     # process shutdown.
-                    main_thread_instance._tstate_lock = threading._allocate_lock()
+                    main_thread_instance._tstate_lock = threading._set_sentinel()
                     main_thread_instance._tstate_lock.acquire()
 
                     # Actually patch the thread ident as well as the threading._active dict

I wonder if there is a way to write a test for this, especially since it only seems to affect PyPy

@rchiodo
Copy link
Contributor

rchiodo commented Sep 26, 2024

I can try a submission. Our tests should validate this code path. Not sure what the difference is between _allocate_lock and _set_sentinel though. Need to do some investigation to see what the functional difference is.

@mattip
Copy link

mattip commented Sep 26, 2024

Thanks

@rchiodo
Copy link
Contributor

rchiodo commented Sep 26, 2024

_set_sentinel isn't part of CPython anymore. This change would have to be predicated on using PyPy only.

I still think PyPy needs to change here. Updated to match CPython? Or maybe there's another change that uses public apis instead of the private _allocate_lock that would fix the problem too.

Oh actually _allocate_lock is just a synonym for this: https://docs.python.org/3/library/_thread.html#thread.allocate_lock, so it is the public API. I think we'd rather use the public api if possible.

@rchiodo
Copy link
Contributor

rchiodo commented Sep 26, 2024

The difference between _set_sentinel and just _allocate_lock is it seems the lock is auto cleaned up when the thread is destroyed. Maybe we can add that to debugpy to cleanup the lock when this thread is destroyed.

@rchiodo
Copy link
Contributor

rchiodo commented Sep 26, 2024

What I'm still not sure of is why we don't see this same crash with CPython. Say CPython 3.11 that looks like the threading code is identical.

@rchiodo
Copy link
Contributor

rchiodo commented Sep 26, 2024

It's also unclear how the mentioned code is actually run. That code should only run on attach AFAIK, @TechPro424 are you doing an attach?

@TechPro424
Copy link

No
Launching with debugpy

@mattip
Copy link

mattip commented Sep 27, 2024

_set_sentinel isn't part of CPython anymore

That changed for the still-unreleased python 3.13, in response to (mainly) an issue in free-threading python/cpython#116372. For earlier versions we are stuck with the "private" thread internals debugpy is overriding in a non-compatible way.

@rchiodo
Copy link
Contributor

rchiodo commented Sep 27, 2024

I tried this myself. That code is not run when doing a launch. Not sure how it worked for @TechPro424. Changing that line has no effect on it crashing for me.

Internally it looks like debugpy doesn't necessarily work with other python runtimes. The work we do around tracing specific threads only works in CPython. I would guess this is much more complicated than a single line change.

@x42005e1f
Copy link

The source of the problem is not on your side. If you replace _allocate_lock() with _set_sentinel() contrary to the comment in fix_main_thread_id(), it will be bad. First, the main thread's life will now be tied to the thread in which you're doing the fix: once the current thread dies, joining the main thread will return. So you're actually undoing the fix that the function author is trying to do. Second, the current thread's own _tstate_lock will no longer work: only the last created sentinel lock will be released when the current thread dies. As a result, joining the thread where you made the fix will never return.

The actual issue was fixed in the py3.10 branch. You can copy threading.py to your PyPy instance's lib/pypy3.10/ (or Lib\ for Windows) and see if that fixes this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs investigation P2
Projects
None yet
Development

No branches or pull requests

8 participants