-
Notifications
You must be signed in to change notification settings - Fork 93
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
Segfault when creating + deserializing snapshot on 0.6.1 #229
Comments
|
Pretty stuck with this one, I have a |
The other thing I noticed that makes me think this is GC related is that the first snapshot doesn't seem to leave behind any MiniRacer::Context at all, while the second snapshot definitely does (That's why I put in the ObjectSpace.to_a in the first place):
EDIT: Looks like I was wrong about this, running the snippet with |
What a pickle, usually when I hit bugs like this I tend to expand the
testing matrix to see if I can isolate to a specific version.
- Can you try on Ruby 2.7/3.0/3.1/latest 2.6 ... does it still happen
there?
- Can you try with latest libv8, I think we are due an upgrade, it is
probably workable to update to latest somehow and do a local install.
…On Fri, Jan 14, 2022 at 2:36 AM nightpool ***@***.***> wrote:
*EDIT:* Looks like I was wrong about this, running the snippet with
GC.disable still causes a crash. Back to the drawing board!
—
Reply to this email directly, view it on GitHub
<#229 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAABIXK3GJ5WVJ6ZZY6VLYLUV3WQVANCNFSM5L2BZSRA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Hi Sam, thanks for the reply. Since I've been reproducing the issue in our local app environment, I haven't been able to try on more recent Ruby versions. (The keyword args backwards incompatibility changes in 2.7 are really chafing). Getting a reproducible test case that works outside of our rails app is next on my list. WRT libv8 versions, our current version is node 16.10.0, with v8 version 9.3.345.19. I've tested on node 16 latest (v8 9.4.146.24) and was able to reproduce the same issue. I've tested on node 17 latest (v8 9.6.180.15) and wasn't able to compile MiniRacer. I'm going to try node 17.1.0 next, which is the previous minor release of v8 (9.5.172.25), to see if that resolves the compilation error. Here's the compilation error I get on 9.6, if you have any idea what could be causing it:
|
I'll be honest, the |
Looks like the rb_rescue2 errors went away after I fixed the |
Unfortunately, I'm running into the same segfault on node v17-latest: Segfault below the cut
|
I just added a patch recently to fix rb_rescue2, does this need to be applied more widely? Segfaulting on init is concerning, is there any process forking involved? Perhaps try single threaded platform to see if this removes the issue? |
I'm already using single threaded platform! See the test case in the OP.
And yes, there is forking involved.
…On Tue, Jan 18, 2022, 11:43 PM Sam ***@***.***> wrote:
I just added a patch recently to fix rb_rescue2, does this need to be
applied more widely?
289587a
<289587a>
Segfaulting on init is concerning, is there any process forking involved?
Perhaps try single threaded platform to see if this removes the issue?
—
Reply to this email directly, view it on GitHub
<#229 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABZCVYXI2EJBMAPTVZAZ4TUWZFQZANCNFSM5L2BZSRA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Perhaps we should add a bit more guards internally to avoid stuff being
called at the wrong time, the api is kind of silent about misuse ... if
platform is already initialized we should explicitly raise an exception if
you try to set it to something else.
…On Wed, Jan 19, 2022 at 4:45 PM nightpool ***@***.***> wrote:
I'm already using single threaded platform! See the test case in the OP.
And yes, there is forking involved.
On Tue, Jan 18, 2022, 11:43 PM Sam ***@***.***> wrote:
> I just added a patch recently to fix rb_rescue2, does this need to be
> applied more widely?
>
> 289587a
> <
289587a
>
>
> Segfaulting on init is concerning, is there any process forking involved?
> Perhaps try single threaded platform to see if this removes the issue?
>
> —
> Reply to this email directly, view it on GitHub
> <#229 (comment)
>,
> or unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/AABZCVYXI2EJBMAPTVZAZ4TUWZFQZANCNFSM5L2BZSRA
>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
>
> or Android
> <
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub
>.
>
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
—
Reply to this email directly, view it on GitHub
<#229 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAABIXJZH5X3X6FZQLGB7VTUWZFYHANCNFSM5L2BZSRA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you commented.Message ID:
***@***.***>
|
I'm not sure I understand what you mean by "the wrong time".... in my test case, I call So i'm pretty sure that i'm using SingleThreadedDefaultPlatform here...... |
@nightpool Do you have a stripped down version of your example? Happy to help it running through different Ruby versions and platforms. |
Hey @nightpool. Are you still dealing with this issue? A couple of mini_racer versions have been released newer V8 versions as well. |
Hi all! Long time no see. I'm trying to upgrade to 0.6.1 to reduce the potential for deadlocks in MiniRacer / V8 (we already use
MiniRacer::Platform.set_flags! :single_threaded
on 0.4.0 / libv8-node 15.14.0.1, but notSingleThreadedDefaultPlatform
). However, after upgrading to 0.6.1, I'm seeing some new segfaults when I attempt to deserialize snapshots.I can pretty reliable reproduce these issues in the console using:
However, we don't see these errors 100% of the time. It seems like after running the above code (which is a very stripped down version of starting up our Unicorn-based React app), the fork call on the last line has about an 85% chance of failing. I've attached the segfault below, the crash appears to happen in
v8::internal::Deserializer::PostProcessNewObject(v8::internal::Handle<v8::internal::Map>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::SnapshotSpace)
We're using Ruby 2.6.8.
The text was updated successfully, but these errors were encountered: