Bug 125228

Summary:

Use ThreadingOnce class to encapsulate pthread_once functionality.

Product:

WebKit

Reporter:

peavo

Component:

JavaScriptCore

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED INVALID

Severity:

Normal

CC:

bfulgham, commit-queue, ggaren

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

125264

Bug Blocks:

Attachments:

Description	Flags
Patch	none

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.