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

Use stable ZeroMQ 6.0.0.beta16 in Jupyter Adapter #551

Closed
jmcphers opened this issue May 8, 2023 · 4 comments
Closed

Use stable ZeroMQ 6.0.0.beta16 in Jupyter Adapter #551

jmcphers opened this issue May 8, 2023 · 4 comments
Labels
area: core Issues related to Core category.

Comments

@jmcphers
Copy link
Collaborator

jmcphers commented May 8, 2023

Currently, Positron uses its own fork of ZeroMQ 6.0.0.beta16 in the Jupyter Adapter.

https://github.com/rstudio/positron/blob/5c7b354ccc1290c58a55327d36d68cbcf7b89d6b/extensions/jupyter-adapter/scripts/install.ts#L24-L37

This is because ZeroMQ 6.0.0.beta16 stable dynamically links to libsodium on macOS, which is a problem for our use case. @kevinushey wrote up an issue here:

zeromq/zeromq.js#557

We should not be using a private fork of ZeroMQ; it slows down compiles and rebuilds, it requires regular maintenance, and it puts us into untested territory.

Some possible ways to address this issue:

  1. Work with the zeromq.js upstream library to get a change incorporated that will stop dynamically linking libsodium.
  2. Publish our own npm package that includes our own fix for the problem (static linking), since there doesn't seem to be much appetite for dealing with this issue upstream
  3. Bundle homebrew's libsodium.dylib in our macOS distribution, and use library paths to cause it to load from our application bundle at runtime. Note that this is the approach espoused by the zeromq maintainers: Fix: Remove libsodium dependency (for now). zeromq/zeromq.js#554 (review)
  4. Go back to using beta.6; figure out why it is incompatible with VS Code >= 1.78.0 and address that instead of trying to use the latest ZeroMQ.
  5. Figure out how the official Jupyter extension handles this problem, and follow the same patterns. At the very least they seem to have some logic for using multiple libraries (zeromq, zeromqold) and resolving zeromq library versions at runtime.
  6. Run away from home and live in the woods.

Appendix 1: How the Jupyter Extension Does It

The Jupyter extension from Microsoft has more or less the same problem we do: they need to bundle ZeroMQ to talk to the Jupyter kernels to power the Notebook feature. Here is a recent change in that area as a toehold:

microsoft/vscode-jupyter#13117

Appendix 2: Crash Symptoms with beta.6

It used to be possible to use an older version of ZeroMQ; prior to merging VS Code 1.78.0, we were using 6.0.0.beta6. However, after picking up 1.78.0, the extension host process began crashing whenever a ZeroMQ API was called. Here is an example of one such crash:

* thread #1, name = 'CrUtilityMain', queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x8)
  * frame #0: 0x000000010c775fc8 zeromq.node`napi_value__* Napi::ObjectWrap<zmq::Socket>::WrappedMethod<&zmq::Socket::GetClosed(Napi::CallbackInfo const&)>(napi_env__*, napi_callback_info__*) + 68
    frame #1: 0x0000000112b0d308 Electron Framework`napi_is_detached_arraybuffer + 296
    frame #2: 0x0000000147e8e618
    frame #3: 0x0000000147e99940
.. snip ...
	frame #39: 0x0000000147e8a908
    frame #40: 0x000000010d989910 Electron Framework`v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) + 2064
    frame #41: 0x000000010d8754ec Electron Framework`v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) + 348
    frame #42: 0x0000000112aabcd8 Electron Framework`node::CallbackScope::~CallbackScope() + 1640
    frame #43: 0x0000000112aac074 Electron Framework`node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context) + 252
    frame #44: 0x0000000112aabf3c Electron Framework`node::MakeCallback(v8::Isolate*, v8::Local<v8::Object>, v8::Local<v8::String>, int, v8::Local<v8::Value>*, node::async_context) + 300
    frame #45: 0x000000010cb5ac20 Electron Framework`v8::CodeEvent::GetScriptName() + 181844
...
@jmcphers jmcphers added this to the Public Beta milestone May 8, 2023
@jmcphers
Copy link
Collaborator Author

jmcphers commented Dec 8, 2023

Now that zeromq/zeromq.js#554 has been merged, we are (should be) able to get off our private ZeroMQ fork. The PR removes libsodium entirely.

We will probably want to wait, however, until the next zeromq.js release so we can use formally released bits.

@jmcphers
Copy link
Collaborator Author

We maybe also be able to excise ZeroMQ from the Jupyter Adapter entirely if we do the kernel gateway work for session support.

@petetronic
Copy link
Collaborator

See #1155

@wesm wesm added the area: core Issues related to Core category. label Feb 29, 2024
@jmcphers
Copy link
Collaborator Author

We have decided to address this by just removing Jupyter Adapter entirely. See #4579.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Issues related to Core category.
Projects
None yet
Development

No branches or pull requests

3 participants