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

Debugger crashes with Python 3 project #402

Closed
PoncinMatthieu opened this issue Aug 27, 2018 · 18 comments
Closed

Debugger crashes with Python 3 project #402

PoncinMatthieu opened this issue Aug 27, 2018 · 18 comments
Labels
lang: python Python rules integration P3 We're not considering working on this, but happy to review a PR. (No assignee) product: IntelliJ IntelliJ plugin stale Issues or PRs that are stale (no activity for 30 days) topic: debugging Issues related to debugging type: bug

Comments

@PoncinMatthieu
Copy link

PoncinMatthieu commented Aug 27, 2018

Summary

We are unable to debug python3 bazel applications when the project sdk is setup to python 3, the debugger will crash with the following error before even executing the application:

Process finished with exit code 137 (interrupted by signal 9: SIGKILL)

If we don't switch the project sdk to python 3, import resolution by intellij will be done using the wrong libs and may be showing unexpected errors.

Steps to reproduce

Clone code example from rules_python repo: https://github.com/bazelbuild/rules_python

Create a venv with python 3, I used 3.6.5 and use the following .bazelproject file:

directories:
  examples/version

targets:
  //examples/version:version_test
  
workspace_type: python

build_flags:
  --python_path=/venv/python3.6.5/bin/python

Debugging will now work if the project sdk is python2, but if you change it to the venv, it will crash.
To change the project sdk, go to File -> Project Structure -> Project

@jin jin added the lang: python Python rules integration label May 1, 2019
@andrewring
Copy link

I'm still seeing this with python 3.6.8.

@mattgodbolt
Copy link

This is a super important feature for us in moving to bazel: if there's a sensible set of debugging information that's needed to look in to this, I'm happy to dig a little. I ran the exact command-line IntelliJ runs without the crash; and strace-ing intellij didn't yield any obvious problems.

@mattgodbolt
Copy link

Some preliminary investigations, trying to wrap my head around debugging python in intellij in intellij :)

  • Seems to trigger when hitting a breakpoint
  • Effects seem "racey" - if I single step through the startup code in BlazePyDebugRunner then sometimes it doesn't crash. BUT in those cases breakpoints aren't caught: it's as if the debug stub hasn't waited for the parent before starting running.

I hope this triggers an "aha" moment with someone following this as I'm out of ideas right now!

@danewhite
Copy link

On Mac, there seems to be a couple issues at play with the os.execv call within Bazel's python_stub_template.txt.

Digging around on the IntelliJ side of things, I encountered this issue with breakpoints not being triggered by IntelliJ following os.execv (note that my workaround for that still triggers SIGKILL in the Bazel scenario):
https://youtrack.jetbrains.com/issue/PY-37960

I haven't dug through BlazePyDebugRunner at all, but my wild guess would be something involving the timing of the exec call being interpreted as a hangup. Like maybe some socket or file handle getting closed (e.g. FD_CLOEXEC).

A not-so-great workaround is to set IntelliJ to use Python 3.3, which has some different os.execv behavior that doesn't trigger any of these bugs. But if you're using any Python 3.4+ syntax or features, then this won't work for you.

If rules_python decides to implement their proposal for customizing python_stub_template.txt, that would allow a potential workaround by switching os.execv to subprocess.call:
https://github.com/bazelbuild/rules_python/blob/master/proposals/2018-11-08-customizing-the-python-stub-template.md

For an invasive workaround as things currently stand, you can modify your IntelliJ pydev code:
~/Library/Application Support/IntelliJIdea2019.1/python/helpers/pydev/_pydev_bundle/pydev_monkey.py

Change this:

def create_execv(original_name):
    def new_execv(path, args):
        """
        os.execv(path, args)
        os.execvp(file, args)
        """
        import os
        if is_python_args(args):
            send_process_will_be_substituted()
        return getattr(os, original_name)(patch_path(path), patch_args(args))
    return new_execv

To this:

def create_execv(original_name):
    def new_execv(path, args):
        """
        os.execv(path, args)
        os.execvp(file, args)
        """
        from subprocess import call
        call(patch_args(args))
    return new_execv

The remaining downside with this approach for me, is that the debugger opens a new tab with the bazel-out version of the file you're debugging.

@danewhite
Copy link

danewhite commented Sep 13, 2019

Dug a little more at the Python 3.3 vs 3.4 differences, and there was a change to default all file descriptors to be non-inheritable, including sockets (see the addition of F_DUPFD_CLOEXEC, and https://docs.python.org/3/library/os.html#fd-inheritance). So it's looking like the socket gets closed when os.exec is called.

So a different invasive workaround within IntelliJ's pydev code, is to just set the client socket to be inheritable:
~/Library/Application Support/IntelliJIdea2019.1/python/helpers/pydev/_pydev_bundle/pydev_comm.py

def start_client(host, port):
    """ connects to a host/port """
    pydevd_log(1, "Connecting to ", host, ":", str(port))

    s = socket(AF_INET, SOCK_STREAM)
    # FIX: Set to inheritable to handle changes in Python 3.4
    if hasattr(s, 'set_inheritable'):
        s.set_inheritable(True)

Hopefully IntelliJ will provide a fix of their own sometime soon, but if not, then maybe something on BlazePyDebugRunner that is able to handle the socket being closed and a new one being opened following the os.exec() call.

@danewhite
Copy link

So the remaining issue on the Bazel plugin side of things, to open the workspace file instead of the bazel-out file, looks like it's just an issue with symlink resolution.

pydev is passing the symlink path back to the IDE when it triggers the breakpoint. This happens within NetCommandFactory.make_thread_suspend_str() found in the same file as the socket workaround:
~/Library/Application Support/IntelliJIdea2019.1/python/helpers/pydev/_pydev_bundle/pydev_comm.py

Note that the function get_abs_path_real_path_and_base_from_frame returns a tuple of:
(os.abspath(file), os.realpath(file), basename(file))

So just swapping the index used by pydev from 0 to 1 is another invasive workaround.

abs_path_real_path_and_base = get_abs_path_real_path_and_base_from_frame(curr_frame)

# Workaround: Changed from 0 to 1 to pass realpath instead of symlinks
my_file = abs_path_real_path_and_base[1]

if is_real_file(my_file):
    # if filename is Jupyter cell id
    # Workaround: Changed from 0 to 1 to pass realpath instead of symlinks
    my_file = norm_file_to_client(abs_path_real_path_and_base[1])

So with the previous hack of pydev start_client() and this one, the IntelliJ debugger appears to be working correctly.

It seems reasonable to have the IntelliJ team fix the socket issue introduced with Python 3.4, but the symlink issue feels like it probably needs to be handled within the Bazel plugin.

Still haven't dug through this code, but I would hope there's somewhere to resolve the symlink that it receives from pydev.

@danewhite
Copy link

danewhite commented Sep 14, 2019

Spent a little time looking at the symlink issue, and it looks like there's code for converting the symlink to the workspace file, but that it isn't being used on the file path it receives from the pydev stack frame:

@SuppressWarnings("MissingOverride") // added in 2019.1 #api183
public PySourcePosition convertPythonToFrame(String filePath, int line) {
return new PySourcePosition(filePath, line) {};
}

So maybe need to wrap the file path with a call to BlazePyPositionConverter.convertFilePath().

I think this is the IntelliJ upstream call into this method, when parsing the pydev stack frame:
https://github.com/JetBrains/intellij-community/blob/be6247932aa9414ddf7831c0e3becba6940f4839/python/pydevSrc/com/jetbrains/python/debugger/pydev/ProtocolParser.java#L215

@mattgodbolt
Copy link

Thanks for your investigations @danewhite. Anyone on the bazel team able to comment if this is the problem, and if so fix? Python2 is about to die and bazel/intellij can't debug python3, which seems a shame (and a blocker to folks like us adopting it across the board)

@sseveran
Copy link

sseveran commented Nov 5, 2019

Any updates? This has become a problem for us as well.

The fix with set_inheritable worked for us so we have a work around to at least get it to run. The debugger seems to be non-functional after that.

@crutcher-dunnavant-techlabs

the set_inheritable() bug still appears to be present

@PoncinMatthieu
Copy link
Author

PoncinMatthieu commented Nov 13, 2020

The latest bazel version 3.7.0 didn't work with these patches, the bazel generated script is using a wrapper to execute the code called "py3wrapper.sh" and was causing the breakpoints to fail.
For anyone interested, I have made a fork of the pydev debugger to incorporate it all here: https://github.com/PoncinMatthieu/PyDev.Debugger/blob/fix/bazel_debugging/README.md

instructions on the readme:

This branch contains a fix for getting the debugger to work with bazel. This fix works with the following versions:

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

Tested only on MacOS.

How to apply the fix?

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

The fix contains one hard coded variable that you may want to change. Located here. This variable defines the python path used to execute your code.

See this for a diff of the path: fabioz/PyDev.Debugger@main...PoncinMatthieu:fix/bazel_debugging

Do note that this fix is very specific to Bazel so I will not attempt to get this merged on the main pydevd repo.

@sseveran
Copy link

@PoncinMatthieu Thanks for such great work. I tested this on Ubuntu 20.04.1 and it worked just fine.

For looking into the future do you have any idea what changes would be needed to the plugin and maybe bazel (specifically rules_python) itself to make a permanent fix? Also I would consider providing some time to making a maintained forked version of the plugin that supports a better python experience. Given the lack of attention these tickets have from Google, they don't internally have the same problems.

@PoncinMatthieu
Copy link
Author

PoncinMatthieu commented Jan 28, 2021

@sseveran sorry for late reply.

Me and other colleagues have been using the fix that I provided in November. While using it, I have found that we still have one bug with the debugger, intellij will crash when trying to do a "step into" action. It hasn't been a big blocker for me as I can just put breakpoints further down when I need to "step into", but it is annoying. I unfortunately haven't had the time to try to fix it. Everything else works well.
Do feel free to use this fork of pydevd. I will probably try to maintain it for future versions of bazel and intellij since this is something I use on a daily basis, I will try to maintain it but I can't promise when I would spend time upgrading those to future versions.

It would indeed be great if we could get a permanent fix. I would say that we have 3 independent fixes in this patch:

  1. Set the socket opened by pydevd as inheritable for the call to os.execv. I think this is something that would be better fixed on the debugger side. However another workaround could be to replace execv with subprocess.call on the Bazel runner. I have now opened a PR to fix this on the PyDev.Debugger repo: set socket as inheritable fabioz/PyDev.Debugger#190

  2. Using real paths to resolve symlinks so that the IDE opens the right file. This is probably something that should be fixed from the debugger as well or maybe even from the Intellij client itself, as it would be best if it could follow symlink. However my fix is very hacky and i'm not familiar enough with the debugger code to make a better one (I was also unable to use the latest version of the debugger found in the repo with this version of intellij i'm using, so I am unable to test any more permanent fix). I don't know how it could be fixed from Bazel other than removing the symlinks.

  3. The wrapper py3wrapper.sh in the bazel generated script is seemingly breaking things too. I'm not sure why this would need to be fixed on bazel side. I also always run bazel on an empty venv to make sure that no system library will impact the build and I haven't found a way to tell bazel to run the code against a specific venv (it would be great if bazel could do that automatically) -- any help on that side would be welcome.

Overall we could probably get all those fixed, 1 and 2 upstreamed to the PyDev.Debugger repo, however what worries me is that I have no clue how the debugger is actually being bundled in the Intellij releases ... they may have their own changes that haven't been upstreamed and I don't know how often they update their version.

@mai93 mai93 added topic: debugging Issues related to debugging type: bug labels Feb 11, 2021
@mrlucasrib
Copy link

I have the same problem with python 3.9

@github-actions
Copy link

Thank you for contributing to the IntelliJ repository! This issue has been marked as stale since it has not had any activity in the last 6 months. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-maintainer". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Mar 16, 2023
@sgowroji sgowroji added product: IntelliJ IntelliJ plugin awaiting-maintainer Awaiting review from Bazel team on issues and removed awaiting-maintainer Awaiting review from Bazel team on issues labels Mar 23, 2023
@github-actions github-actions bot removed the stale Issues or PRs that are stale (no activity for 30 days) label Mar 24, 2023
@keertk keertk moved this from Untriaged to Triaged in Bazel IntelliJ Plugin Apr 17, 2023
@mai93 mai93 added the P3 We're not considering working on this, but happy to review a PR. (No assignee) label Apr 17, 2023
@mai93 mai93 moved this from Triaged to Backlog in Bazel IntelliJ Plugin Apr 17, 2023
@mai93 mai93 removed their assignment Apr 17, 2023
@jdimatteo
Copy link

This is fixed in at least some version combinations of intellij/bazel/plugin versions, is there documentation of this being fixed somewhere?

Copy link

github-actions bot commented Mar 8, 2024

Thank you for contributing to the IntelliJ repository! This issue has been marked as stale since it has not had any activity in the last 6 months. It will be closed in the next 14 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Mar 8, 2024
Copy link

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please post @bazelbuild/triage in a comment here and we'll take a look. Thanks!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 22, 2024
@github-project-automation github-project-automation bot moved this from Backlog to Done in Bazel IntelliJ Plugin Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang: python Python rules integration P3 We're not considering working on this, but happy to review a PR. (No assignee) product: IntelliJ IntelliJ plugin stale Issues or PRs that are stale (no activity for 30 days) topic: debugging Issues related to debugging type: bug
Projects
Development

No branches or pull requests