[feature request] Make C-extension threadsafe/Ractor-safe

[mohamedhafez]

In planning ahead to the near future when TruffleRuby can run C-extensions marked with rb_ext_ractor_safe(true) in parallel, and for when Ractors are no longer just experimental, it would be great if the C-extension could be made threadsafe, or marked as such if it already is so!

[stevecheckoway]

Libxml2 doesn't support concurrent modifications the same document. See https://gitlab.gnome.org/GNOME/libxml2/-/wikis/Thread-safety

[mohamedhafez]

So the way Ractors work is that only one of them can access a given document object at a time, so libxml2's limitation of not supporting concurrent modifications on the same document actually shouldn't be an issue: https://ruby-doc.org/core-3.0.0/Ractor.html

What I'm hoping to avoid is that accessing different document objects can't be done concurrently, which is currently the case. According to the link you posted, libxml2 explicitly allows this as long as you:

• configure the library accordingly using the --with-threads options

• call xmlInitParser() in the "main" thread before using any of the libxml2 API (except possibly selecting a different memory allocator)

So I'm hoping this actually should be trivial!

(I'm addressing the use case of Ractors only here, since thats the only way it would happen in canonical, regular C-Ruby. Thread.new and Fibers are still subject to the GVL in C-Ruby, and Ractors are the only way to do true concurrency. TruffleRuby and JRuby users already know to protect access to the same object with a Mutex if they are doing multithreaded programming, and if they don't they are going to be screwed in a million other places;)

[eregon]

• --with-threads seems already included:
  

  nokogiri/ext/nokogiri/extconf.rb

  Line 949 in d992447

  "--with-threads",

  
  I'm not sure about --without-tls (on the next line) and whether we'd need to remove that.
• xmlInitParser() is already called during Init_ so that's fine:

  nokogiri/ext/nokogiri/nokogiri.c

  Line 197 in d992447

  xmlInitParser();

So I think we can just install current nokogiri as it is, and run it with some threads on TruffleRuby with --cexts-lock=false and see whether that works to get an idea.

[mohamedhafez]

@eregon perhaps you or someone on the TruffleRuby team could lend a little more gravitas to my argument above? 😅

[flavorjones]

@mohamedhafez Thanks for opening this issue. Earlier this year I spent some time exploring how ractors and the sqlite3 gem interact, so I have questions.

Have you tried parsing and manipulating documents in different ractors? What was your experience like? What worked and what didn't work?

Our mental model is that although libxml2 doesn't support concurrent operations within a single document, each ractor should be able to parse and manipulate a separate document, and I'd like to update our mental model if your experience has been something different.

When you say "support for ractors" I'm trying to understand your specific use case, and what specific error message motivated you to open this issue. Passing objects between ractors can be hard for complex object graphs, and so any additional information you can provide would help me form better mental models.

[flavorjones]

As an aside: coincidentally, the upstream "make libxml2 thread-safe" epoch just got closed today: https://gitlab.gnome.org/GNOME/libxml2/-/issues/407

[mohamedhafez]

@flavorjones so the mental model you mentioned, of each ractor should be able to parse and manipulate a separate document, is exactly what I'm hoping for. Currently, if you try to use Nokogiri in a Ractor, it will fail with a Ractor::UnsafeError error, because the C-extention hasn't been marked as Ractor-safe with rb_ext_ractor_safe(true). To reproduce:

~ $ curl 'https://nokogiri.org/tutorials/installing_nokogiri.html' > /tmp/installing_nokogiri.html
~ $ irb
3.3.3 :001 > require 'nokogiri'
 => true 
3.3.3 :002 > Ractor.new { puts Nokogiri::HTML(File.open("/tmp/installing_nokogiri.html")).inspect }
(irb):2: warning: Ractor is experimental, and the behavior may change in future versions of Ruby! Also there are many implementation issues.
 => #<Ractor:#2 (irb):2 blocking> 
#<Thread:0x000000011f3d31d0 run> terminated with exception (report_on_exception is true):
/Users/mohamed/.rvm/gems/ruby-3.3.3/gems/nokogiri-1.16.6-arm64-darwin/lib/nokogiri/html4/document.rb:194:in `read_io': ractor unsafe method called from not main ractor (Ractor::UnsafeError)
	from /Users/mohamed/.rvm/gems/ruby-3.3.3/gems/nokogiri-1.16.6-arm64-darwin/lib/nokogiri/html4/document.rb:194:in `parse'
	from /Users/mohamed/.rvm/gems/ruby-3.3.3/gems/nokogiri-1.16.6-arm64-darwin/lib/nokogiri/html4.rb:11:in `HTML4'
	from (irb):2:in `block in <top (required)>'

This is the expected behavior: https://docs.ruby-lang.org/en/3.3/extension_rdoc.html#label-Appendix+F.+Ractor+support. According to that doc, the fix basically boils down to make sure you protect access to global variables with a Mutex, and make sure any external libraries like libxml2 are safe to access from different threads, and then call rb_ext_ractor_safe(true) in the Init_ function. We are already guaranteed that individual document objects can't be accessed by two different Ractors at the same time, as that's a basic feature of a Ractor

[flavorjones]

Yeah, OK. There are some complicating factors, just to make it clear that this isn't as easy as flipping a switch.

We still need to support old versions of libxml2 (for users who choose to use their system libraries) that don't have many of the thread-safety improvements mentioned in the libxml2 issue I linked above, which means we need to be very careful about when and how we mark the extension as ractor-safe.

Because we're still supporting older versions, we haven't migrated Nokogiri to use the new features everywhere because doing so would mean having more complex code in many places. So there's still quite a bit of work that would need to be done within Nokogiri to be Ractor-safe even with new versions of libxml2.

The roadmap for the next year is currently structured like:

• v1.17.0 (imminent) bug fixes, lots of bug fixes
• v1.18.0 (Dec 2024) performance, support for Ruby 3.4
• v2.0.0 (early 2025) breaking API changes

I've been planning to look into Ractor support for v1.18.0, but I'm not going to guarantee anything since support is still a function of

• what versions of libxml2 we decide to support
• whether we decide to conditionally use all the new thread-safety features with modern versions of libxml2
• how long it takes to support the new thread-safety features
• my judgement in how complex all of the above is, and whether I think it's really "safe" to use ractors
• if there are bugs found in the new thread-safety features, latency to communicate with upstream and get them fixed

I'm not convinced this should be our highest priority right now, even in the realm of performance. There are quite a few things we think we can do that would improve the single-thread use cases (see the label topic/performance to get a sense) that I think should take priority. Also consider that Ractors are still experimental, and this is the first time anybody's asked for Ractor support in Nokogiri.

If I'm being honest, there's been more interest in updating Nokogiri to release the GVL when it calls into libxml2, and that may be something we choose to do before we address Ractor safety. I just don't know right now.

I might consider marking the gem as Ractor-safe and calling it "experimental" to get more user feedback. But I would want to build at least some confidence in our approach first.

Anyway. Thanks for your patience.

[flavorjones]

@mohamedhafez If you're interested in working on this, a great place to start would be to patch a development version of Nokogiri and turn on ractor safety and tell us what happens. We learn 10x more from breaking things than we do having long prose conversations about how we think things might work! ❤️

[mohamedhafez]

Ah damn didn't realize the situation with the older versions of libxml2 made it so complicated, I was hoping it would be as simple as flipping a switch :P Totally understand prioritizing single-thread performance and a whole host of other things before this, thats 99.9% of your users' use cases. As far as me working on it, I haven't written anything in C since freshman year 20 years ago haha, and just to be honest I don't have the bandwith to take on new projects just in general I'm sorry!

(I don't actually have any code written using Ractors that I could test out a patched Nokogiri on, and this little reproduction is the first I've ever used them at all actually. My project is in JRuby, and in the course of evaluating whether I could run it on TruffleRuby, TruffleRuby's inability to run non-Ractor-safe C extensions concurrently is a big blocker because certain gems I use heavily aren't Ractor-safe)

[mohamedhafez]

@flavorjones, following @eregon reporting in #3283 (reply in thread) that nokogiri already configures libxml to be multithreaded, I've been running my test workload on TruffleRuby with the C-extension lock turned off, and no issues as far as I can tell!

I've got 10 threads cycling through 50 jobs (each job consists of downloading a webpage and processing it with Nokogiri to pick out a bunch of info from it). Then there's a 1 second pause, then I repeat. I've had that running for a couple hours now and no problems!

[mohamedhafez]

Considering that making nokogiri Ractor-safe, or releasing the GVL before going into libxml2, both would require significant work that would be a ways off, perhaps a more easily achievable goal would be getting to the point where we could declare rb_ext_thread_safe(true), so that TruffleRuby could run libxml2 & the other nokogiri C-extension parts in parallel.

The reason this would be easier is we wouldn't have to worry about re-acquiring the GVL when libxml2 or the c-extension callback into Ruby, since TruffleRuby already runs Ruby without the GVL by default. The latest versions of nokogiri already run libxml2 with --with-threads, so we wouldn't have to worry about libxml2 itself, just nokogiri's c files themselves. (Question: even though --with-threads is passed, are older versions of libxml2 that don't support this option still supported by nokogiri, like they were in the past? If so, is there a way to require an up to date version just for TruffleRuby?)

Searching through the codebase, I'm seeing many usages of static variables in c files, though i'm not sure if maybe they are all just static read-only constants. Unfortunately I don't have the time or really anything but the most rudimentary understanding of C, so I wouldn't be able to really audit the codebase to see if this would work, I'm just hoping to start the discussion here (sorry!!) Obviously understandable if others don't have the time either, just leaving this here and crossing my fingers!

Oh also: note that in experiments I've run on my own codebase with TruffleRuby with the C-extension lock turned off, I didn't have any issues and everything worked as expected (see previous comment), so I am hoping there might not actually be many changes needed, if any!

[flavorjones]

I don't have plans to work on this myself in the immediate future, but would welcome anyone with the time and motivation to start working on it.