RESOLVED FIXED 20914
[QT] JavaScriptCore segmentation fault
https://bugs.webkit.org/show_bug.cgi?id=20914
Summary [QT] JavaScriptCore segmentation fault
Peter Gal
Reported 2008-09-18 05:49:04 PDT
After r36541 when running JavaScriptCore it exits with segmentation fault. The problem seems to be the QCoreApplication::instance() call wich returns null.
Attachments
proposed patch (1.43 KB, patch)
2008-09-18 05:51 PDT, Peter Gal
timothy: review+
typo fixed patch (1.43 KB, patch)
2008-09-19 05:28 PDT, Peter Gal
eric: review+
Peter Gal
Comment 1 2008-09-18 05:51:05 PDT
Created attachment 23528 [details] proposed patch
Peter Gal
Comment 2 2008-09-19 05:28:52 PDT
Created attachment 23563 [details] typo fixed patch The previous patch had a typo, which made a header to be skipped if the platform is a MAC. It was tested only on linux/qt so this is why I missed it.
Eric Seidel (no email)
Comment 3 2008-09-21 14:47:57 PDT
Comment on attachment 23563 [details] typo fixed patch I'm going to flip the meaning of the second #if when I land, so that it's more clear the code is only used for DARWIN.
Eric Seidel (no email)
Comment 4 2008-09-21 14:54:16 PDT
Landed as r36745, thanks for the patch!
Alexey Proskuryakov
Comment 5 2008-09-22 00:58:03 PDT
I don't see why this fix is correct - won't this just keep crashing on Darwin, if it did on other platforms? I think you can just use "mainThreadIdentifier = currentThread();" everywhere (although I'm not sure why the variable is needed on Darwin at all).
Peter Gal
Comment 6 2008-09-22 01:18:17 PDT
I couldn't tested it on any other platform than linux. I looked webkit's buildbot and there the mac version didn't show any errors while the linux version clearly crashed. This is the main reason why I think this patch is ok. Don't really now why was it changed in r36541, but I did't wanted to remove that code, maybe there will be some future plans whit it?
Tor Arne Vestbø
Comment 7 2008-09-22 01:38:14 PDT
Yes, kjs will still crash on Darwin. If I understood you correct Alexey, JSC::initializeThreading() can be called from non-main threads on Darwin, so I used QCoreApplication to get the main thread instead of currentThread(), so that mainThreadIdentifier will always be pointing to the main thread no matter which thread the init was called from. What I didn't foresee was that the jsc binary does not init QCoreApplication. I'll look into it. Thanks for the notice :)
Tor Arne Vestbø
Comment 8 2008-09-22 06:30:25 PDT
Landed in r36763
Alexey Proskuryakov
Comment 9 2008-09-22 07:23:56 PDT
(In reply to comment #7) > If I understood you correct Alexey, JSC::initializeThreading() can be called > from non-main threads on Darwin At the moment, the contract is that JSC::initializeThreading() should be called from the main thread. One exception is Mac OS X JavaScriptCore API clients, and that's because some of these shipped before we put this in place. So, for the Qt port JSC::initializeThreading() should be called from the main thread, even on Darwin. You are free to see this as an unfortunate limitation, and lift it some day, of course. I'm not sure that the workaround with calling QCoreApplication is so great - besides our own test shell, there may be other command line apps or daemons that want to use JSC, and creating a full QCoreApplication my be inappropriate for some. But at least it gets us going for now.
Note You need to log in before you can comment on or make changes to this bug.