Skip to content
This repository has been archived by the owner on Jan 15, 2021. It is now read-only.

test-regress-GH-4015 regression on Ubuntu using SpiderMonkey #22

Closed
enricogior opened this issue Aug 10, 2016 · 5 comments
Closed

test-regress-GH-4015 regression on Ubuntu using SpiderMonkey #22

enricogior opened this issue Aug 10, 2016 · 5 comments
Assignees
Milestone

Comments

@enricogior
Copy link
Member

enricogior commented Aug 10, 2016

Test test-regress-GH-4015.js fails on Ubuntu 16.04 using SpiderMonkey.

The test forces a stack overflow and then checks the error signal.
The test passes on Travis running Ubuntu 12.04 and it fails on our dev machines running Ubuntu 16.04.
In our environment the err.signal is set to 'null' instead of the expected 'SIGSEGV'.

=== release test-regress-GH-4015.js ===
Path: _auto_simple_single/test-regress-GH-4015.js
AssertionError: false == true ( 0:0)
    at ok (assert.js:126:5)
    at /home/enrico/github/thaliproject/jxcore/test/_auto_simple_single/test-regress-GH-4015.js:21:5
    at exithandler (child_process.js:
@yaronyg
Copy link
Member

yaronyg commented Aug 23, 2016

Diego has set up an environment to let us debug both C and Javascript. We are seeing that spider monkey isn't catching the exception which it should but even then JXcore should handle it (and does on OS/X) but for some reason it doesn't on Linux. It might be related to jxcore#563 since we think the same mechanism is at play even if the cause is different.

@yaronyg
Copy link
Member

yaronyg commented Aug 24, 2016

Each platform has its own signal handler so it turns out that Android and Linux share one, Mac has one, Windows has one, but iOS doesn't have one. The part that runs on Linux and Android has some very scary comments in it. Different versions of Android they have different ways of sending signals and they don't catch all the signals. There are comments in there that on some Android systems the signal system always passes a null pointer so they don't always get the data about what went wrong. Enrico and Diego are seeing this problem. Diego has investigated and is checking version 38 and 45 of SpiderMonkey to see if there is a fix. Our version is 34 from 12/2014. 38 has the same structure and different code, 45 is completely different, a full rewrite. So right now Diego is investigating 38 to see if he can backport any fix. It will take another couple of points to investigate and understand. This is going to hit us on Android. We need to be able to determine exactly what type of access violation because in some cases Spidermonkey can handle the error itself and in some cases it needs to crash but the logic is a bit of a mess and inconsistent right now.

@yaronyg
Copy link
Member

yaronyg commented Aug 25, 2016

Diego has made excellent progress. We hope to work around the problem. They added an exception handler for segmentation fault signal in JXcore. For example the test that was failing had two parts, one part had a child process that ran the actual test. If we ran that script directly jxcore crashes. We now know how to fix it. The exception handler they are adding is done right and is only used on Linux and Android. But they know now how to avoid JXcore crashing. The other thing is that spider monkey has a problem where the exception handler is trying to get a pointer to a stack and that pointer is always null. We checked all the threads and it is null in all the threads so it's always null. We don't know if that is a bug in spider monkey or a bug in the initialization code when JXcore initializes spider monkey or what. But at least we know where the problem is and if we want to fix it we potentially can. For now it isn't clear if we need to fix it since we have the work around to catch it in JXcore.

@enricogior
Copy link
Member Author

Moving this issue to the Ready column since for now Diego has finished the investigation.
The fix will require to add a signal handler for SIGSEGV in jxcore.
We hold off committing the fix while investigating thaliproject/Thali_CordovaPlugin#563 to see if the same fix apply.

@enricogior
Copy link
Member Author

PR submitted #49

@yaronyg yaronyg closed this as completed Sep 6, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants