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

fix: namespace nodes behave with compaction #2658

Merged
merged 6 commits into from Oct 16, 2022

Conversation

flavorjones
Copy link
Member

What problem is this PR intended to solve?

Under certain circumstances, GC compaction will cause a segfault when accessing wrapped XML::Namespace objects.

See previous similar work at #2579

This fix, if merged, should be backported to v1.13.x and released as a bugfix.

Have you included adequate test coverage?

Yes, repro test is included thanks to @eightbitraptor and @peterzhu2118

Does this change affect the behavior of either the C or the Java implementations?

No functional behavior changes.

@flavorjones flavorjones requested a review from tenderlove Oct 6, 2022
ext/nokogiri/xml_namespace.c Outdated Show resolved Hide resolved
@flavorjones
Copy link
Member Author

flavorjones added 3 commits Oct 16, 2022
I haven't used it in years, TBH, and what it does is completely
insufficient to debug memory issues in modern Ruby anyway.
@flavorjones flavorjones force-pushed the flavorjones-namespace-scopes-compaction branch from e1dd979 to d42abaa Compare Oct 16, 2022
@flavorjones
Copy link
Member Author

🤯 So fixing compaction for namespaces has caught a long-standing use-after-free bug in in the Document#remove_namespaces! method. Just pushed a fix for that.

flavorjones and others added 3 commits Oct 16, 2022
@flavorjones flavorjones force-pushed the flavorjones-namespace-scopes-compaction branch from d42abaa to 13c8aaa Compare Oct 16, 2022
@flavorjones flavorjones merged commit ee2066e into main Oct 16, 2022
112 checks passed
@flavorjones flavorjones deleted the flavorjones-namespace-scopes-compaction branch Oct 16, 2022
@flavorjones flavorjones added the topic/memory Segfaults, memory leaks, valgrind testing, etc. label Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/memory Segfaults, memory leaks, valgrind testing, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants