matsadler / magnus

Ruby bindings for Rust. Write Ruby extension gems in Rust, or call Ruby from Rust.
https://docs.rs/magnus/latest/magnus/
MIT License
682 stars 35 forks source link

Deadlock when first creating rust-ext objects on threads #109

Closed choubacha closed 6 months ago

choubacha commented 6 months ago

Magnus can cause a deadlock when creating the same type of instance on multiple threads. I've gone ahead and reproduced this issue in a separate repo which can be referenced. The reproduction must be run several times to hit the timing of it although it's possible a sleep in the Lazy once block might help cause it to hit every time.

The reason this happens is due to how rb_protect works when calling into a closure. When it calls out it may choose to yield control back to ruby which may then activate a different thread. rb_protect is called when initialization the lazy type in magnus is called. This can create a competition for two locks. One is the Once lock that the code block is running and the other lock is the GVL.

choubacha commented 6 months ago

Adding in a sleep to the Lazy code does indeed cause the deadlock to hit every time. I've added this in as well.

matsadler commented 6 months ago

Ooof. Thank you for reporting this, and thanks so much for the reproduction. I'm sure this one was a tricky one to track down, so I really appreciate the effort.

I fixed this in fb833092e39badc98d6c5934ad0edbb8c94facfe, and back ported to the 0.6 branch too.

I am planning to put out a 0.6.4 with the fix, but CI is failing*, and I want to try and figure out what's up with that before doing the release.

* weirdly just for macOS arm, Ruby 3.1.5, Rust 1.78, which I can actually test locally and it works just fine, so probably just GitHub's runners being weird

matsadler commented 6 months ago

I published the 0.6.4 release. I narrowed down the CI failure to a feature Magnus' tests rely on, but users generally won't touch, so I don't think it's worth blocking the release on.

choubacha commented 6 months ago

Thank you! I'll get this integrated right away! :) I really appreciate your quick turnaround on this.

choubacha commented 6 months ago

I cannot reproduce it now, thank you again!