Bug 175124 - [Linux][WTF] Use one global semaphore to notify thread suspend and resume completion
Summary: [Linux][WTF] Use one global semaphore to notify thread suspend and resume com...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-08-03 03:46 PDT by Yusuke Suzuki
Modified: 2017-08-03 04:18 PDT (History)
9 users (show)

See Also:

Attachments
Patch (8.02 KB, patch)
2017-08-03 03:50 PDT, Yusuke Suzuki 		cgarcia: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2017-08-03 03:46:21 PDT 
[Linux][WTF] Use one global semaphore to notify thread suspend and resume completion
Comment 1 Yusuke Suzuki 2017-08-03 03:50:42 PDT 
Created attachment 317109 [details]
Patch
Comment 2 Yusuke Suzuki 2017-08-03 03:53:12 PDT 
Comment on attachment 317109 [details]
Patch

ok, ready.
Comment 3 Carlos Garcia Campos 2017-08-03 04:10:23 PDT 
Comment on attachment 317109 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=317109&action=review

> Source/WTF/wtf/ThreadingPthreads.cpp:89
> +    Semaphore(unsigned initialValue)

explicit?

> Source/WTF/wtf/ThreadingPthreads.cpp:92
> +        int sharedBetweenProcesses = 0;
> +        sem_init(&m_platformSemaphore, sharedBetweenProcesses, initialValue);

Do we really need a local variable? or is it to document what that 0 means?

> Source/WTF/wtf/ThreadingPthreads.cpp:202
> +    globalSemaphoreForSuspendResume.construct(0);

This is the only place it's constructed and we always pass 0 as initial value, right?
Comment 4 Yusuke Suzuki 2017-08-03 04:13:20 PDT 
Comment on attachment 317109 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=317109&action=review

>> Source/WTF/wtf/ThreadingPthreads.cpp:89
>> +    Semaphore(unsigned initialValue)
> 
> explicit?

Yeah, nice. Fixed.

>> Source/WTF/wtf/ThreadingPthreads.cpp:92
>> +        sem_init(&m_platformSemaphore, sharedBetweenProcesses, initialValue);
> 
> Do we really need a local variable? or is it to document what that 0 means?

Yes, it is to document what that 0 means.

>> Source/WTF/wtf/ThreadingPthreads.cpp:202
>> +    globalSemaphoreForSuspendResume.construct(0);
> 
> This is the only place it's constructed and we always pass 0 as initial value, right?

Right. initializePlatformThreading() is called from initializeThreading(). And its content is guarded by call_once.
Thus, this construction only happens once. And it should be done before using Threading. (initializeThreading() calls this function and constructs this semaphore.).
Comment 5 Yusuke Suzuki 2017-08-03 04:17:49 PDT 
Committed r220201: <http://trac.webkit.org/changeset/220201>
Comment 6 Radar WebKit Bug Importer 2017-08-03 04:18:23 PDT 
<rdar://problem/33697497>