WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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+
Details
Formatted Diff
Diff
typo fixed patch
(1.43 KB, patch)
2008-09-19 05:28 PDT
,
Peter Gal
eric
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
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.
Top of Page
Format For Printing
XML
Clone This Bug