Skip to content

Conversation

@JafarAbdi
Copy link

Fix: pappasam/jedi-language-server#347

See https://github.com/JafarAbdi/jedi_bug_reproduce for how to reproduce the bug.

Note: I used claude code to help debug and fix this issue.

Redirect file descriptor 1 to /dev/null in addition to sys.stdout. This prevents compiled C extensions (like pyorbbecsdk) that write directly to fd 1 from corrupting the pickle stream used for IPC.

Also catch pickle.UnpicklingError alongside EOFError for graceful error handling when stream corruption does occur.
@davidhalter
Copy link
Owner

Thanks for trying to contribute @JafarAbdi. I feel like this is not a bad patch, but I'm not sure I trust it in all cases. That's why I'm probably not going to merge. It's hard to test stuff like this and I don't want to work with something that can break so quickly. I hope I don't disappoint you, but this is a very rare thing and I think in most cases people should probably fix the extensions.

@PeterJCLaw
Copy link
Collaborator

@davidhalter are you able to clarify/quantify your concerns here?

I agree that stdout redirection like this isn't ideal, however the failure modes I can think of are only cases where the current issue essentially remains -- either due to something else managing to get the handle before it's been redirected or (very unlikely) where something gets the new handle somehow.

Is your concern for example that this doesn't go far enough and that you'd like to e.g: use sockets or named pipes for IPC instead? (I can see an argument in favour of that as it would also simplify debagging for example)

@JafarAbdi
Copy link
Author

Thanks for trying to contribute @JafarAbdi. I feel like this is not a bad patch, but I'm not sure I trust it in all cases. That's why I'm probably not going to merge. It's hard to test stuff like this and I don't want to work with something that can break so quickly. I hope I don't disappoint you, but this is a very rare thing and I think in most cases people should probably fix the extensions.

Thank you for looking into the PR. I totally understand. I'll keep it open in case other people find it useful

@davidhalter
Copy link
Owner

davidhalter commented Dec 21, 2025

Is your concern for example that this doesn't go far enough and that you'd like to e.g: use sockets or named pipes for IPC instead? (I can see an argument in favour of that as it would also simplify debagging for example)

I'm personally a big fan of stdout/stin for IPC, but it's obviously a bit worrying when we have arbitrary code execution that might write stdout or read from stdin.

I think my main concern is just that this is incomplete and lacks code that I cannot verify, because I'm not very knowledgeable about files especially not on Windows, where this might just behave slightly different. I don't think it's a big issue, since almost nobody just writes to stdout in extensions (for no good reason). I also don't think it helps that AI is involved, since I have seen it fail in spectacular and unexpected ways.

However if you can verify and think that this is a good patch, feel free to merge. I'm not at all against that. I feel like you're very competent and you might be more capable in verifying if this is in a good shape. Feel free to do whatever with this patch :)

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.

jedi_language_server: -32603: _pickle.UnpicklingError: could not find MARK

3 participants