RFC: Stop exporting symbols

[flavorjones]

This document is a request for comments, specifically from downstream maintainers of gems that integrate with Nokogiri's C API, or the C API of libxml2, libxslt, or libgumbo.

Proposal

Limit the symbols that are exported by the Nokogiri shared library (extension) to the bare minimum necessary for Nokogiri to operate. This might be the single symbol Init_nokogiri, but I need to do some exploration to be sure.

Context

Nokogiri has always exported the symbols from libxml2 and libxslt that are statically linked into the extensions. On Linux this has led to one or two issues with symbol collisions (see #2106), but with careful naming we've mostly avoided problems.

In the final days of shipping Nokogiri v1.14.0 with native (precompiled) support for Ruby 3.2, we're struggling a bit with symbol resolution. Ruby 3.2, when compiling on Darwin, now uses the -bundle_loader linker flag to resolve symbols against the Ruby executable as if it were a shared library. This means that, when running a Ruby compiled with the --enable-shared flag, that the extension will fail to resolve Ruby symbols like rb_cObject. We can work around that with the -flat_namespace linker flag, which mimics the behavior we already see on Linux and allows us to resolve these symbols at runtime. But for reasons I don't fully understand, many Rubies on Darwin seem to load the libxml2 and libxslt dylibs that ship with XCode commandline tools ("CLT"), and so every libxml2 symbol is a collision and resolves to the wrong libxml2 (not the version we've patched and statically linked into the extension).

To work around this last problem, the best solution we know of right now seems to be to avoid exporting those symbols by using the -load_hidden flag (or a similar mechanism, there are several we could choose from).

The decision I think we've made is to avoid exporting these symbols only in the precompiled Ruby 3.2 Darwin extensions. But that opens up the question of: should we avoid exporting these symbols everywhere, both for consistency and for reliability. That is the question asked by this RFC.

Impact

Good: This would prevent accidental symbol collisions such as the #2106 on Linux, and would ensure that we always pull in the desired version of libxml2, avoiding problems like the ones we're currently experiencing with Ruby 3.2 (see rake-compiler/rake-compiler-dock#87 for extended discussion and more links).

Bad: This would also, however, prevent a small but non-zero number of downstream gems from integrating with Nokogiri's C API, or the C API of libxml2, libxslt, or libgumbo. A notable gem that did this was https://github.com/rubys/nokogumbo (now merged into Nokogiri itself). Another notable gem that I know that does this is nokogiri-xmlsec (and the various forks of it, the most popular seems to be https://github.com/instructure/nokogiri-xmlsec-instructure). So this may prevent experimentation and innovation (see Nokogumbo) as well as putting hurdles in front of useful integrations like xmlsec.

Feedback

I would like to hear from downstream gem maintainers about how they're using the C APIs exported by Nokogiri's extension today. Please comment below!

References

• nokogiri/2022-12-darwin-symbol-resolution.md at main · sparklemotion/nokogiri · GitHub
• ruby/ruby@50d81bf
• [Ruby 3.2] having runtime issues on darwin · Issue #87 · rake-compiler/rake-compiler-dock
• dep: add ruby 3.2 support by flavorjones · Pull Request #2732 · sparklemotion/nokogiri
• https://github.com/stevecheckoway/bundle_test and explore whether `-load_hidden` will work around flat namespace by flavorjones · Pull Request #1 · stevecheckoway/bundle_test
• Video from Apple explaining the Darwin toolchain changes

[flavorjones]

I'm trying out this approach with the sqlite3-gem in this draft PR: sparklemotion/sqlite3-ruby#371

[flavorjones]

Note for posterity: it worked, and this is now how sqlite3-ruby ships.

[maths22]

https://github.com/instructure/nokogiri-xmlsec-instructure currently uses noko_xml_document_pin_node and noko_xml_node_wrap, which if i'm understanding this proposal correctly would go away under the proposal. It does rely on libxml symbols as well, but I believe gets those from the linker loading libxmlsec1's dependencies, so it doesn't depend on nokogiri for them. (I am neither the original author of that extension nor very experienced at ruby C extension development, so I am definitely far from an expert on how that gem works or if it is doing things optimally. I did however inherit maintenance of it, so I do care about trying to prevent it from breaking.)

[flavorjones]

For posterity, here's what the Nokogumbo merger planning looked like: #2204

[flavorjones]

One more note, just to clarify how I'm thinking about making these decisions. I have an assumption that there are very few gems that are linking against Nokogiri's libxml2 or libxslt symbols (C functions).

It feels likely that this number may in fact be 1 (nokogiri-xmlsec) in which case, that's a compelling reason to consider merging it into Nokogiri. This number may be higher -- and that would be surprising -- and in that case I would want to try harder to come up with a solution to meet all those gems' needs.

But in 14 years of maintaining Nokogiri, I only remember seeing two gems that linked against Nokogiri's libxml2: nokogumbo and nokogiri-xmlsec. If that holds true, that is a good indicator that we don't need to support exporting symbols.

[maths22]

Running this search path:ext/**/*.c noko_ -path:*/nokogiri/* in github codesearch returns only nokogiri-xmlsec-instructure and forks, which I would say is at least a good argument in favor of your hypothesis. And I think it would be a lot stranger if we had a gem that linked against nokogiri's exported libxml symbols without linking against any of against nokogiri's own symbols, but people do surprising things sometimes.

[maths22]

Oh I missed the first message of your three. I forgot to mention that my local test was with ruby 3.2. I'd definitely be happy to help support the libxmlsec functionality in nokogiri. I know what my company uses it for and why we use it, but I'd also need to do some digging to really properly understand how it does what it does; so far mostly I just touch whatever breaks on a ruby or nokogiri upgrade and move on with my life, so I can't claim to understand it well. But given that we rely on its functionality for hundreds of thousands of saml logins a day it would probably be worthwhile getting to know it better :)

[stevecheckoway]

@maths22 there’s a decent chance that Nokogiri’s vendored libxml2 and whichever libxml2 is getting linked to Ruby (and that nokogiri-xmlsec is linking to) are in fact different versions of libxml2. They could be incompatible versions. I wouldn’t be shocked if there are subtle bugs as a result, particularly around line numbers. (Libxml2’s design assumes at most 65534 lines but has a compile-time hack to kinda support larger line numbers in some cases.)

I spent a while trying to figure out how to deal with this on macOS by getting the runtime linker, dyld, to do the right thing. I didn’t really succeed.

One option is using dlopen/dlsym to look up specific symbols. (I investigated a hand-rolled GOT/PLT-like approach—essentially caching the symbol per call-site using a macro. It worked but the programming experience wasn’t great.)

One thing I’d like to understand is all of the configurations we want to support and then see how we can make that work on all supported platforms. There are many tricky restrictions around (re-)exporting symbols, two-level namespaces, vendored vs. system libraries, bundles vs. dynamic libraries, and —enable-shared vs. —disable-shared Ruby versions. Trying to support everything is probably too painful. And that’s just macOS. ELF symbol resolution is different. I need to do a bunch of reading about symbol resolution on Windows to know how that differs from the others.

[flavorjones]

OK, I think we've made a decision, which is:

• we'll plan to not export libxml2/libxslt symbols in an upcoming minor release
• I'd like to integrate the nokogiri-xmlsec features into Nokogiri before (or in) that release

Obviously we can still learn as we do this work, and may possible change direction, but I'd like to get started.

[flavorjones]

Fun and interesting twist: #2898 shows that problems happen to other gems when they accidentally pull in Nokogiri's libxml2.

This increases my confidence that we've chosen the right path in deciding to not export symbols in an upcoming minor release. Ugh, what a mess.

[flavorjones]

Also note that the YARP project just ran into this issue, and I proposed solving it by hiding symbols in ruby/prism#1069

[flavorjones]

And here's a followup commit I made to YARP introducing symbol visibility tests: ruby/prism@c87d91a

We will want to do something similar for Nokogiri