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

pp.Server() error #296

Closed
Xin-yang-Liu opened this issue Mar 10, 2020 · 12 comments
Closed

pp.Server() error #296

Xin-yang-Liu opened this issue Mar 10, 2020 · 12 comments
Assignees
Labels
bug Something isn't working

Comments

@Xin-yang-Liu
Copy link

Issue Type: Bug

everytime I debug python code:

import pp
pp.Server

It will raise the error message:
Exception has occurred: RuntimeError
Communication pipe read error

Extension version: 2020.2.64397
VS Code version: Code 1.43.0 (78a4c91400152c0f27ba4d363eb56d2835f9903a, 2020-03-09T19:44:52.965Z)
OS version: Linux x64 5.3.0-40-generic

System Info
Item Value
CPUs Intel(R) Xeon(R) Gold 6138 CPU @ 2.00GHz (40 x 1000)
GPU Status 2d_canvas: unavailable_software
flash_3d: disabled_software
flash_stage3d: disabled_software
flash_stage3d_baseline: disabled_software
gpu_compositing: disabled_software
multiple_raster_threads: enabled_on
oop_rasterization: disabled_off
protected_video_decode: disabled_off
rasterization: disabled_software
skia_renderer: disabled_off_ok
video_decode: disabled_software
viz_display_compositor: enabled_on
viz_hit_test_surface_layer: disabled_off_ok
webgl: unavailable_software
webgl2: unavailable_software
Load (avg) 1, 1, 1
Memory (System) 62.33GB (54.85GB free)
Process Argv --no-sandbox --unity-launch
Screen Reader no
VM 0%
@int19h
Copy link
Contributor

int19h commented Jun 11, 2020

This is a fairly old version of the extension (and, consequently, the debugger). Can you try it with the most recent version?

@Xin-yang-Liu
Copy link
Author

This is a fairly old version of the extension (and, consequently, the debugger). Can you try it with the most recent version?

I tried with the extension v2020.5.86806, it raises the same error message

@int19h
Copy link
Contributor

int19h commented Jun 15, 2020

Thank you for checking! Can you provide more info on your environment, specifically the Python version, version of Parallel Python you're using, and how you've installed it.

Also, can you please add "logToFile": true to your launch.json, and share the log files (debugpy*.log) that it produces in your extension folder when you try to debug?

@int19h int19h transferred this issue from microsoft/vscode-python Jun 16, 2020
@int19h int19h added the bug Something isn't working label Jun 16, 2020
@Xin-yang-Liu
Copy link
Author

Python version is 3.7.6. Parallel Python version is 1.6.4.4, installed by downloading package and installed manually.
Here is the log file debugpy.adapter-140882.log

@int19h
Copy link
Contributor

int19h commented Jun 17, 2020

It looks like it's a long-standing known issue with pp and debuggers, e.g.: https://stackoverflow.com/questions/41120464/parallel-python-communication-pipe-read-error

It appears that this has something to do with subprocess debugging. Can you try this in your launch.json:

"subProcess": false

and see if that helps?

@int19h
Copy link
Contributor

int19h commented Jun 17, 2020

@fabioz, here's the code in pp that creates the subprocess in question:

class _Worker(object):
    """Local worker class
    """
    command = [sys.executable, "-u", "-m", "ppworker"]

    command.append("2>/dev/null")

    def __init__(self, restart_on_free, pickle_proto):
        """Initializes local worker"""
        self.restart_on_free = restart_on_free
        self.pickle_proto = pickle_proto
        self.start()

    def start(self):
        """Starts local worker"""
        if _USE_SUBPROCESS:
            proc = subprocess.Popen(self.command, stdin=subprocess.PIPE,
                    stdout=subprocess.PIPE, stderr=subprocess.PIPE)
            self.t = pptransport.CPipeTransport(proc.stdout, proc.stdin)
        else:
            self.t = pptransport.CPipeTransport(
                    *popen2.popen3(self.command)[:2])

       #open('/tmp/pp.debug', 'a+').write('Starting _Worker\n')
       #open('/tmp/pp.debug', 'a+').write('receiving... \n')
        self.pid = int(self.t.receive())
       #open('/tmp/pp.debug', 'a+').write('received: %s\n' % self.pid)
       #open('/tmp/pp.debug', 'a+').write('sending: %s\n' % self.pickle_proto)
        self.t.send(str(self.pickle_proto))
       #open('/tmp/pp.debug', 'a+').write('...sent \n')
        self.is_free = True

    def stop(self):
        """Stops local worker"""
        self.is_free = False
        self.t.send('EXIT') # can send any string - it will exit
        self.t.close()


    def restart(self):
        """Restarts local worker"""
        self.stop()
        self.start()

    def free(self):
        """Frees local worker"""
        if self.restart_on_free:
            self.restart()
        else:
            self.is_free = True

I see two weird things here. The first one is that it's trying to use 2> to redirect stderr, even though it's not using the shell to spawn the process - I don't see how this is supposed to work at all, but potentially that's something that command line rewriting might be breaking.

The other thing is that it uses -u Python switch to ensure that child process output is unbuffered. Would that be propagated correctly?

@Xin-yang-Liu
Copy link
Author

It looks like it's a long-standing known issue with pp and debuggers, e.g.: https://stackoverflow.com/questions/41120464/parallel-python-communication-pipe-read-error

It appears that this has something to do with subprocess debugging. Can you try this in your launch.json:

"subProcess": false

and see if that helps?

It does not work, the error remains the same.

@fabioz
Copy link
Collaborator

fabioz commented Jun 18, 2020

I've investigated it here and the subprocess attaches fine (that 2>/dev/null ends up in the sys.argv for the new process and doesn't actually redirect anything from the shell and the -u is also passed properly to the new python executable).

The actual problem is that parallel-python has the following code:

class PipeTransport(Transport):
    def __init__(self, r, w):
        if isinstance(r, types.FileType) and isinstance(w, types.FileType):
            self.r = r
            self.w = w
        else:
            raise TypeError("Both arguments of PipeTransport constructor " \
                    "must be file objects")

which is called by:

pptransport.CPipeTransport(sys.stdin, sys.__stdout__)

And because the debugger patches sys.stdin it throws that TypeError in the subprocess, which breaks the communication (with the RuntimeError: Communication pipe read error).

I'm not sure about the best approach... if parallel-python was being actively developed I'd say that the fix should be in the library as usually it's expected that clients use duck-typing in Python (it's explicitly being hostile from changes to sys.stdin, even if they would work properly). The fix in parallel python would be checking if the class has the needed attributes instead of checking for the actual instance (it just ends up using read(int) and close() anyways), but even though it says it's open source I can't find a repo for its code and the last release (on https://pypi.org/project/pp/) is from 2016, so, it seems like a dead project.

Given that, I think a fix in the debugger may be reasonable... I think it should be possible to make DebugConsoleStdIn a subclass of file on Python 2 and io.IOBase on Python 3 to make pp believe it's dealing with the proper class.

fabioz added a commit to fabioz/debugpy that referenced this issue Jun 18, 2020
@fabioz
Copy link
Collaborator

fabioz commented Jun 18, 2020

Humm, I tried the fix I had in mind but it breaks the patching (right now the patching works well because we have a __getattr__ which returns the original methods when needed, but if we start to subclass the patch is no longer ideal because it will actually have those methods).

We could override all those methods, but the solution isn't as good as what we have now (we end up playing a cat and mouse game whenever Python itself is updated and the methods available change).

One thing I noted is that parallel-python already does use sys.__stdout__ (as it knows it goes hard against monkey-patching) but keeps using sys.stdin instead of sys.__stdin__.

My suggestion is to fix that in the library in your local version to use sys.__stdin__ too:

self.t = pptransport.CPipeTransport(sys.__stdin__, sys.__stdout__) -- instead of the current self.t = pptransport.CPipeTransport(sys.stdin, sys.__stdout__).

in pp-1.6.4.4/python3/ppworker.py and try to talk to the library maintainers to fix that in the library itself (I know the lib looks dead, so, if they can't fix it, my suggestion is to fork it and use your own fork).

Another option could be not doing any patching at all or have a flag for it... we do it to notify the client that an input is being requested (so, for instance, if the client uses a non real tty he can show a dialog to ask for input or redirect anything typed in some console to the stdin -- right now we haven't gotten to that in VSCode, so, an option could be just saying that this will never be supported and don't monkey-patch it at all or make it a user-preference).

@int19h @karthiknadig what do you think?

@int19h
Copy link
Contributor

int19h commented Jun 18, 2020

Are sys.stdin and sys.stdout still monkey-patched if "redirectOutput" is set to false in the debug config?

@fabioz
Copy link
Collaborator

fabioz commented Jun 18, 2020

Are sys.stdin and sys.stdout still monkey-patched if "redirectOutput" is set to false in the debug config?

Only sys.stdin, but I guess having that be controlled by redirectOutput does make sense. I'll change it so that it respects that setting.

@fabioz
Copy link
Collaborator

fabioz commented Jul 17, 2020

I'll start to work on only redirecting sys.stdin if redirectOutput==true.

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

No branches or pull requests

3 participants