Bug 125228 - Use ThreadingOnce class to encapsulate pthread_once functionality.
Summary: Use ThreadingOnce class to encapsulate pthread_once functionality.
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 125264
Blocks:
  Show dependency treegraph
 
Reported: 2013-12-04 10:45 PST by peavo
Modified: 2014-01-06 11:55 PST (History)
3 users (show)

See Also:


Attachments
Patch (1.79 KB, patch)
2013-12-04 10:53 PST, peavo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description peavo 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.
Comment 1 peavo 2013-12-04 10:53:31 PST
Created attachment 218416 [details]
Patch
Comment 2 Brent Fulgham 2013-12-04 12:13:46 PST
Comment on attachment 218416 [details]
Patch

r=me
Comment 3 WebKit Commit Bot 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>
Comment 4 WebKit Commit Bot 2013-12-04 12:40:33 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Geoffrey Garen 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.
Comment 6 WebKit Commit Bot 2013-12-04 15:59:00 PST
Re-opened since this is blocked by bug 125264
Comment 7 Brent Fulgham 2013-12-04 16:00:30 PST
We can switch to "std::call_once" when we move to VS2013.
Comment 8 peavo 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.
Comment 9 peavo 2014-01-06 11:55:59 PST
This is now fixed by using std::call_once.