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

set socket as inheritable #190

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

PoncinMatthieu
Copy link

@PoncinMatthieu PoncinMatthieu commented Jan 28, 2021

Setting the client socket as inheritable would fix issues with the debugger when the application creates a child process with os.execv. This behavior used to work prior to python 3.4, since python 3.4, sockets are by default not inheritable.
Link to the relevant python doc: https://docs.python.org/3/library/os.html#fd-inheritance

More specifically, this fix will allow us to debug bazel programs with the bazel intellij plugin. Thread here: bazelbuild/intellij#402

Edit: tests are failing but I'm not sure why, I'd be grateful if anyone could help with fixing those and merging the PR.

@fabioz
Copy link
Owner

fabioz commented Jan 28, 2021

Humm, something else seems off... by default when a new process is spawned, a new connection from the debugger to the client should be done instead of reusing a socket (so, I'm surprised making it inheritable actually works at all).

Do you know why that makes it work? Do you think it'd be possible to have a test-case which reproduces the issue (without Bazel in the middle?)

Note: the (few) failing tests are due to one unrelated issue (GitHub is having some issues on providing gdb which is needed for these tests).

@PoncinMatthieu
Copy link
Author

ok I will try to reproduce it without Bazel in the middle and with the latest versions. If I manage to do so, I will look into writing a test case.

I've been using an older version of the debugger (as the latest version from main didn't work with the version of intellij compatible with the bazel plugin)

bazel: 3.7.0
intellij: 2020.1.4
intellij bazel plugin: 2020.10.19.0.3
pydevd: this branch is based on the tag 1.7.1, newer versions didn't work with intellij

so I wonder if it's possible the new connection behavior you described was added after 1.7.1 ?

I am not sure also if the debugger that is bundled with intellij is the exact same one on this repo, this was the procudure I used, let me know if that's complete non-sense:

cd ~/Library/Application\ Support/JetBrains/IntelliJIdea2020.1/plugins/python/helpers/
mv pydev pydev_save
git clone [email protected]:PoncinMatthieu/PyDev.Debugger.git pydev

@fabioz
Copy link
Owner

fabioz commented Jan 28, 2021

Humm, that seems a bit dangerous... I think Intellij has a fork which they update from time to time with this one. I don't know if they made any change on their own repo which they haven't upstreamed (which could potentially break their client implementation).

@PoncinMatthieu
Copy link
Author

@fabioz I am indeed able to reproduce this.

Here is a minimal file to reproduce it with:

import sys, os

print("running main")
if len(sys.argv) == 1:
    os.execv(sys.executable, [sys.executable] + sys.argv + ["1"])

I will get this output with the patch:

running main
running main

Process finished with exit code 0

However without the patch I get always a keyboard interrupt:

running main
Failed to import the site module
Traceback (most recent call last):
  File "/Users/ponpon/.pyenv/versions/3.6.5/lib/python3.6/site.py", line 544, in <module>
    main()
  File "/Users/ponpon/.pyenv/versions/3.6.5/lib/python3.6/site.py", line 527, in main
    known_paths = venv(known_paths)
  File "/Users/ponpon/.pyenv/versions/3.6.5/lib/python3.6/site.py", line 464, in venv
    addsitepackages(known_paths, [sys.prefix])
  File "/Users/ponpon/.pyenv/versions/3.6.5/lib/python3.6/site.py", line 328, in addsitepackages
    addsitedir(sitedir, known_paths)
  File "/Users/ponpon/.pyenv/versions/3.6.5/lib/python3.6/site.py", line 207, in addsitedir
    addpackage(sitedir, name, known_paths)
  File "/Users/ponpon/.pyenv/versions/3.6.5/lib/python3.6/site.py", line 168, in addpackage
    exec(line)
  File "<string>", line 1, in <module>
KeyboardInterrupt

Process finished with exit code 1

The second process always receives a KeyboardInterrupt when the socket isn't set as inheritable.

I'm testing on OSX, the following showed the same results:

  • latest Intellij (2020.3.2) + python 3.6.5 + intellij bundled pydevd
  • latest Intellij (2020.3.2) + python 3.9.1 + intellij bundled pydevd
  • latest Eclipse (2020-12) + python 3.9.1 + latest pydevd from http://www.pydev.org/updates (8.1.0.202012051215)
    Without patching the debugger and setting the socket as inheritable, I will get a single line running main.

However when I switch to python 3.3.7, it works as intended, printing 2 lines and confirming the regression from python 3.4.

@PoncinMatthieu
Copy link
Author

PoncinMatthieu commented Jan 29, 2021

I now tried to create a test case for it, but I am now stuck. The test fail as it is not breaking a second time on the same line during the second execution of the program. I'm not sure what I am missing, any pointers? Should the writer be reconnected somehow?

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.

2 participants