Bug 125228

Summary: Use ThreadingOnce class to encapsulate pthread_once functionality.
Product: WebKit Reporter: peavo
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: bfulgham, commit-queue, ggaren
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 125264    
Bug Blocks:    
Attachments:
Description Flags
Patch none

peavo
Reported 2013-12-04 10:45:44 PST
In runtime/InitializeThreading.cpp, the non-pthread version is not fully equivalent to the pthread version. We can use the ThreadingOnce class here.
Attachments
Patch (1.79 KB, patch)
2013-12-04 10:53 PST, peavo 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
peavo
Comment 1 2013-12-04 10:53:31 PST
Created attachment 218416 [details] Patch
Brent Fulgham
Comment 2 2013-12-04 12:13:46 PST
Comment on attachment 218416 [details] Patch r=me
WebKit Commit Bot
Comment 3 2013-12-04 12:40:31 PST
Comment on attachment 218416 [details] Patch Clearing flags on attachment: 218416 Committed r160116: <http://trac.webkit.org/changeset/160116>
WebKit Commit Bot
Comment 4 2013-12-04 12:40:33 PST
All reviewed patches have been landed. Closing bug.
Geoffrey Garen
Comment 5 2013-12-04 15:24:39 PST
This is wrong. Please roll it out. It is incorrect for WTF::ThreadingOnce to be declared static. Since we build without thread-safe statics, that declaration is not thread-safe. Also, if statics were thread-safe, there would be no need for WTF::ThreadingOnce to contain any logic or data members: the constructor could just call the provided function, and you could rely on the compiler to ensure that the constructor was called only once. Therefore, this patch is either meaningless or wrong, depending on your platform.
WebKit Commit Bot
Comment 6 2013-12-04 15:59:00 PST
Re-opened since this is blocked by bug 125264
Brent Fulgham
Comment 7 2013-12-04 16:00:30 PST
We can switch to "std::call_once" when we move to VS2013.
peavo
Comment 8 2013-12-05 07:45:19 PST
(In reply to comment #5) > This is wrong. Please roll it out. > > It is incorrect for WTF::ThreadingOnce to be declared static. Since we build without thread-safe statics, that declaration is not thread-safe. > > Also, if statics were thread-safe, there would be no need for WTF::ThreadingOnce to contain any logic or data members: the constructor could just call the provided function, and you could rely on the compiler to ensure that the constructor was called only once. > > Therefore, this patch is either meaningless or wrong, depending on your platform. Thanks for catching this. A couple of other places needs to be changed as well, I've made bug 125305 for this. Sorry for the inconvienience.
peavo
Comment 9 2014-01-06 11:55:59 PST
This is now fixed by using std::call_once.
Note You need to log in before you can comment on or make changes to this bug.