a new instance of ThreadPriviate is created in function createThreadInternal() (ThreadingQt.cpp), but no place takes care of the deletion. See code below. ThreadIdentifier createThreadInternal(ThreadFunction entryPoint, void* data, const char*) { ThreadPrivate* thread = new ThreadPrivate(entryPoint, data); ...
Created attachment 45280 [details] fix patch
Attachment 45280 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 JavaScriptCore/wtf/qt/ThreadingQt.cpp:183: static_cast is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 1
Haha. That's a lame false positive from the style bot. I'll file a bug.
My apologies. This is my mistake. I posted a patch to fix this false alarm. Thanks Adam for filing this bug.
Created attachment 45450 [details] fix patch to take care both synchronous and asynchronous mode
style-queue ran check-webkit-style on attachment 45450 [details] without any errors.
Comment on attachment 45450 [details] fix patch to take care both synchronous and asynchronous mode This looks good to me as detachThread() could be called from any thread including "self" thread, but the ThreadPrivate destructor is called from the main application thread. LGTM, r=me.
Comment on attachment 45450 [details] fix patch to take care both synchronous and asynchronous mode Clearing flags on attachment: 45450 Committed r52550: <http://trac.webkit.org/changeset/52550>
All reviewed patches have been landed. Closing bug.
REOPEN as The change regressed the following LayoutTests for QtWebKit. fast/workers/worker-call.html -> crashed fast/workers/worker-close.html -> crashed Reverted as http://trac.webkit.org/changeset/52551.
Created attachment 45563 [details] fix crash in patch 2 calling deleteLater in detachThread may not guarantee the thread is finished while being deleted. Using a slot function to capture the finished() signal and calling deleteLater from there is safe.
Attachment 45563 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 JavaScriptCore/wtf/qt/ThreadingQt.cpp:72: Place brace on its own line for function definitions. [whitespace/braces] [4] JavaScriptCore/wtf/qt/ThreadingQt.cpp:78: Place brace on its own line for function definitions. [whitespace/braces] [4] JavaScriptCore/wtf/qt/ThreadingQt.cpp:291: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 3
Created attachment 45564 [details] fix style check fixed the first 2 style checks. I think the 3rd one is a kind of false alarm. Qt requires to put "#include *.moc" at the end of the cpp file. I ran the check-webkit-style script on some existing files and got the same complains.
Attachment 45564 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 JavaScriptCore/wtf/qt/ThreadingQt.cpp:293: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 1
Comment on attachment 45564 [details] fix style check Looks good, r=me
Comment on attachment 45564 [details] fix style check Clearing flags on attachment: 45564 Committed r52602: <http://trac.webkit.org/changeset/52602>