Bug 220110 – Add a helper method to WTF::MachSemaphore to wait with a timeout duration

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Wenson Hsieh]

Namely, the ability to broadcast (semaphore_signal_all), and the ability to wait for a limited amount of time before giving up (semaphore_timedwait)

[Wenson Hsieh]

Created attachment 416730 [details]
WIP

[Wenson Hsieh]

Created attachment 416823 [details]
Patch

[Sam Weinig]

Comment on attachment 416823 [details]
Patch

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

> Source/WTF/wtf/cocoa/MachSemaphore.cpp:62
> +void MachSemaphore::waitWithTimeout(Seconds timeout)

We call this waitFor() in WTF::Semaphore and WTF::BinarySemaphore, perhaps we should keep the names the same?

> Source/WTF/wtf/cocoa/MachSemaphore.cpp:64
> +    auto seconds = static_cast<unsigned>(timeout.seconds());

I think you can use timeout.secondsAs<unsigned>() here and below instead of explicitly casting yourself.

[Wenson Hsieh]

Comment on attachment 416823 [details]
Patch

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

>> Source/WTF/wtf/cocoa/MachSemaphore.cpp:62
>> +void MachSemaphore::waitWithTimeout(Seconds timeout)
> 
> We call this waitFor() in WTF::Semaphore and WTF::BinarySemaphore, perhaps we should keep the names the same?

Sounds good — changed to waitFor().

>> Source/WTF/wtf/cocoa/MachSemaphore.cpp:64
>> +    auto seconds = static_cast<unsigned>(timeout.seconds());
> 
> I think you can use timeout.secondsAs<unsigned>() here and below instead of explicitly casting yourself.

timeout.secondsAs<unsigned>() ought to work. However, timeout.nanosecondsAs<unsigned>() won't work for anything above a few seconds. I can use timeout.nanosecondsAs<int64_t>() and then subtract against the number of nanoseconds, but I'll still need a static cast to clock_res_t around the result.

[Wenson Hsieh]

Created attachment 416825 [details]
Patch

[Wenson Hsieh]

Comment on attachment 416825 [details]
Patch

Thanks for the review!

[EWS]

Committed r271101: <https://trac.webkit.org/changeset/271101>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416825 [details].

[Radar WebKit Bug Importer]

<rdar://problem/72697931>

[Darin Adler]

Comment on attachment 416825 [details]
Patch

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

> Source/WTF/wtf/cocoa/MachSemaphore.cpp:65
> +    auto seconds = timeout.secondsAs<unsigned>();
> +    semaphore_timedwait(m_semaphore, { seconds, static_cast<clock_res_t>(timeout.nanosecondsAs<uint64_t>() - seconds * NSEC_PER_SEC) });

Seems like the "convert Seconds to a mach_timespec_t" should be a separate function; could be useful elsewhere and a good separate concept. Simpler but related to MonotonicTime::fromMachAbsoluteTime.