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

FFI::MemoryPointer autorelease issue #6284

Closed
ahorek opened this issue Jun 15, 2020 · 4 comments · Fixed by #6331
Closed

FFI::MemoryPointer autorelease issue #6284

ahorek opened this issue Jun 15, 2020 · 4 comments · Fixed by #6331

Comments

@ahorek
Copy link
Contributor

ahorek commented Jun 15, 2020

Environment Information
jruby 9.3.0.0-SNAPSHOT (2.6.5) 2020-06-15 ccecbe8 Java HotSpot(TM) 64-Bit Server VM 11.0.2+9-LTS on 11.0.2+9-LTS +jit [linux-x86_64]
(and jruby 9.2.11.1)

sassc 2.4.0

recently, I've noticed segfaults on https://travis-ci.org/github/rails/sprockets/jobs/696412377

git clone https://github.com/rails/sprockets.git
bundle update
rake test

Expected Behavior
it looks like FFI::MemoryPointer is being GCed, even if autorelease is explicitly disabled

Actual Behavior
see sass/sassc-ruby#205

*** Error in `/usr/local/lib/jvm/openjdk11/bin/java': double free or corruption (out): 0x00007efc4cde2550 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7efc554f37e5]
/lib/x86_64-linux-gnu/libc.so.6(+0x8037a)[0x7efc554fc37a]
/lib/x86_64-linux-gnu/libc.so.6(cfree+0x4c)[0x7efc5550053c]
[0x7efc34c67890]
======= Memory map: ========
00400000-00401000 r-xp 00000000 08:01 791938                             /usr/local/lib/jvm/openjdk11/bin/java
...
@kares kares added this to the JRuby 9.3.0.0 milestone Jun 30, 2020
@headius
Copy link
Member

headius commented Jul 16, 2020

Aha, well this certainly could be a cause of #6310 and sass/sassc-ruby#208.

@headius headius modified the milestones: JRuby 9.3.0.0, 9.2.13.0 Jul 16, 2020
headius added a commit to headius/jruby that referenced this issue Jul 16, 2020
This appears to be long-broken logic for setting autorelease to
false, since the only value that will change the unmanaged bit is
passing autorelease = true. The change here allows toggling both
ways.

This may not have been seen before because this version of
MemoryIO is only used when the requested size is greater than 256
bytes, and typically structs are smaller than that. In jruby#6284, as
well as jruby#6310 and sass/sassc-ruby#208, we have reports of a double
free, and sassc itself uses autorelease on a potentially large
value here:

https://github.com/sass/sassc-ruby/blob/4bd764f568ad312a78181d7a3187f3715388e33e/lib/sassc/native.rb#L54-L58
@headius
Copy link
Member

headius commented Jul 16, 2020

@ahorek Help me by trying out #6331

@headius headius linked a pull request Jul 16, 2020 that will close this issue
@ahorek
Copy link
Contributor Author

ahorek commented Jul 16, 2020

patch #6331 confirmed. thank you!

@headius
Copy link
Member

headius commented Jul 16, 2020

@ahorek Hot diggity! We'll roll it into 9.2.13.

@headius headius closed this as completed Jul 16, 2020
enebo pushed a commit that referenced this issue Aug 31, 2020
This appears to be long-broken logic for setting autorelease to
false, since the only value that will change the unmanaged bit is
passing autorelease = true. The change here allows toggling both
ways.

This may not have been seen before because this version of
MemoryIO is only used when the requested size is greater than 256
bytes, and typically structs are smaller than that. In #6284, as
well as #6310 and sass/sassc-ruby#208, we have reports of a double
free, and sassc itself uses autorelease on a potentially large
value here:

https://github.com/sass/sassc-ruby/blob/4bd764f568ad312a78181d7a3187f3715388e33e/lib/sassc/native.rb#L54-L58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants