Skip to content

Commit 42354e4

Browse files
flavorjonesflavorjones
authored
Merge pull request #2243 from sparklemotion/flavorjones-v1_11_x-update-tests-to-work-with-system-libxml-2_9_12
(v1.11.x) update tests to work with system libxml 2.9.12, and work around windows dll unloading issue
2 parents 4b9bfe3 + 05f30eb commit 42354e4

5 files changed

Lines changed: 88 additions & 5 deletions

File tree

‎.github/workflows/windows.yml‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# this is a work in progress!
2+
name: windows
3+
on:
4+
push:
5+
branches:
6+
- main
7+
pull_request:
8+
types: [opened, synchronize]
9+
branches:
10+
- '*'
11+
12+
jobs:
13+
windows:
14+
name: "windows, sys: ${{ matrix.sys }}, ${{ matrix.ruby }}"
15+
16+
env:
17+
MAKEFLAGS: -j2
18+
19+
runs-on: windows-latest
20+
21+
strategy:
22+
fail-fast: false
23+
matrix:
24+
sys: [ enable, disable ]
25+
ruby: [ "2.5", "2.6", "2.7", "3.0", "mingw" ]
26+
27+
steps:
28+
- name: configure git crlf on windows
29+
run: |
30+
git config --system core.autocrlf false
31+
git config --system core.eol lf
32+
- name: checkout
33+
uses: actions/checkout@v2
34+
- name: load Ruby and bundle install
35+
uses: MSP-Greg/setup-ruby-pkgs@v1
36+
with:
37+
ruby-version: ${{ matrix.ruby }}
38+
mingw: libxml2 libxslt
39+
bundler-cache: true
40+
- uses: actions/cache@v2
41+
if: matrix.sys == 'disable'
42+
with:
43+
path: ports/archives
44+
key: ${{ matrix.os }}-${{ matrix.ruby }}-tarballs-${{ hashFiles('**/dependencies.yml') }}
45+
restore-keys: ${{ matrix.os }}-${{ matrix.ruby }}-tarballs-
46+
- name: bundle exec rake compile
47+
run: |
48+
bundle exec rake compile -- --${{ matrix.sys }}-system-libraries
49+
- name: bundle exec rake test
50+
run: bundle exec rake test

‎CHANGELOG.md‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,22 @@ Nokogiri follows [Semantic Versioning](https://semver.org/), please see the [REA
44

55
---
66

7+
## 1.11.5 / 2021-05-19
8+
9+
### Fixed
10+
11+
[Windows CRuby] Work around segfault at process exit on Windows when using libxml2 system DLLs.
12+
13+
libxml 2.9.12 introduced new behavior to avoid memory leaks when unloading libxml2 shared libraries (see [libxml/!66](https://gitlab.gnome.org/GNOME/libxml2/-/merge_requests/66)). Early testing caught this segfault on non-Windows platforms (see [#2059](https://github.com/sparklemotion/nokogiri/issues/2059) and [libxml@956534e](https://gitlab.gnome.org/GNOME/libxml2/-/commit/956534e02ef280795a187c16f6ac04e107f23c5d)) but it was incompletely fixed and is still an issue on Windows platforms that are using system DLLs.
14+
15+
We work around this by configuring libxml2 in this situation to use its default memory management functions. Note that if Nokogiri is not on Windows, or is not using shared system libraries, it will will continue to configure libxml2 to use Ruby's memory management functions. `Nokogiri::VERSION_INFO["libxml"]["memory_management"]` will allow you to verify when the default memory management functions are being used. [[#2241](https://github.com/sparklemotion/nokogiri/issues/2241)]
16+
17+
18+
### Changed
19+
20+
`Nokogiri::VERSION_INFO["libxml"]` now contains the key `"memory_management"` to declare whether libxml2 is using its `default` memory management functions, or whether it uses the memory management functions from `ruby`. See above for more details.
21+
22+
723
## 1.11.4 / 2021-05-14
824

925
### Security

‎ext/nokogiri/nokogiri.c‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,26 @@ Init_nokogiri()
191191
rb_const_set(mNokogiri, rb_intern("OTHER_LIBRARY_VERSIONS"), NOKOGIRI_STR_NEW2(NOKOGIRI_OTHER_LIBRARY_VERSIONS));
192192
#endif
193193

194+
#if defined(_WIN32) && !defined(NOKOGIRI_PACKAGED_LIBRARIES)
195+
/*
196+
* We choose *not* to do use Ruby's memory management functions with windows DLLs because of this
197+
* issue in libxml 2.9.12:
198+
*
199+
* https://github.com/sparklemotion/nokogiri/issues/2241
200+
*
201+
* If the atexit() issue gets fixed in a future version of libxml2, then we may be able to skip
202+
* this config only for the specific libxml2 versions 2.9.12.
203+
*
204+
* Alternatively, now that Ruby has a generational GC, it might be OK to let libxml2 use its
205+
* default memory management functions (recall that this config was introduced to reduce memory
206+
* bloat and allow Ruby to GC more often); but we should *really* test with production workloads
207+
* before making that kind of a potentially-invasive change.
208+
*/
209+
rb_const_set(mNokogiri, rb_intern("LIBXML_MEMORY_MANAGEMENT"), NOKOGIRI_STR_NEW2("default"));
210+
#else
211+
rb_const_set(mNokogiri, rb_intern("LIBXML_MEMORY_MANAGEMENT"), NOKOGIRI_STR_NEW2("ruby"));
194212
xmlMemSetup((xmlFreeFunc)ruby_xfree, (xmlMallocFunc)ruby_xmalloc, (xmlReallocFunc)ruby_xrealloc, ruby_strdup);
213+
#endif
195214

196215
xmlInitParser();
197216

‎lib/nokogiri/version/info.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ def to_hash
137137
else
138138
libxml["source"] = "system"
139139
end
140+
libxml["memory_management"] = Nokogiri::LIBXML_MEMORY_MANAGEMENT
140141
libxml["iconv_enabled"] = libxml2_has_iconv?
141142
libxml["compiled"] = compiled_libxml_version.to_s
142143
libxml["loaded"] = loaded_libxml_version.to_s

‎test/html/test_comments.rb‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,7 @@ class TestComment < Nokogiri::TestCase
113113
let(:subject) { doc.at_css("div#under-test") }
114114
let(:inner_div) { doc.at_css("div#do-i-exist") }
115115

116-
if Nokogiri.uses_libxml? && Nokogiri::VersionInfo.instance.libxml2_using_packaged?
117-
# see patches/libxml2/0006-htmlParseComment-treat-as-if-it-closed-the-comment.patch
116+
if Nokogiri::VersionInfo.instance.libxml2_using_packaged? || (Nokogiri::VersionInfo.instance.libxml2_using_system? && Nokogiri.uses_libxml?(">=2.9.11"))
118117
it "behaves as if the comment is normally closed" do # COMPLIANT
119118
assert_equal 3, subject.children.length
120119
assert subject.children[0].comment?
@@ -128,9 +127,7 @@ class TestComment < Nokogiri::TestCase
128127
end
129128
end
130129

131-
if Nokogiri.jruby? || Nokogiri::VersionInfo.instance.libxml2_using_system?
132-
# this behavior may change to the above in libxml v2.9.11 depending on whether
133-
# https://gitlab.gnome.org/GNOME/libxml2/-/merge_requests/82 is merged
130+
if Nokogiri.jruby? || (Nokogiri::VersionInfo.instance.libxml2_using_system? && Nokogiri.uses_libxml?("<2.9.11"))
134131
it "behaves as if the comment encompasses the inner div" do # NON-COMPLIANT
135132
assert_equal 1, subject.children.length
136133
assert subject.children.first.comment?

0 commit comments

Comments
 (0)