Bug 238596 - [Linux] Implement IPC::Semaphore
Summary: [Linux] Implement IPC::Semaphore
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: Zan Dobersek
URL:
Keywords: InRadar
Depends on:
Blocks: 238593
  Show dependency treegraph
 
Reported: 2022-03-31 01:18 PDT by Zan Dobersek
Modified: 2022-04-01 10:45 PDT (History)
3 users (show)

See Also:


Attachments
Patch (6.30 KB, patch)
2022-03-31 01:19 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (6.43 KB, patch)
2022-04-01 04:36 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2022-03-31 01:18:44 PDT
[Linux] Implement IPC::Semaphore
Comment 1 Zan Dobersek 2022-03-31 01:19:10 PDT
Created attachment 456214 [details]
Patch
Comment 2 Adrian Perez 2022-04-01 01:43:59 PDT
Comment on attachment 456214 [details]
Patch

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

> Source/WebKit/Platform/IPC/unix/IPCSemaphoreUnix.cpp:40
> +    m_fd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK | EFD_SEMAPHORE);

TIL eventfd() can be used as a semaphore, and the functionality is actually
there since Linux 2.6.30 🤯️, and also supported since NetBSD 10.

This might be an issue at some point for other BSDs which provide
WebKitGTK packages in their ports collections; but I say let's not
worry about that for now--surely their developers will find a way,
and/or we can help them out a bit when the time comes.

> Source/WebKit/Platform/IPC/unix/IPCSemaphoreUnix.cpp:73
> +        return;

IIUC, reaching this with an invalid file descriptor means that the caller
is trying to use a destroyed semaphore, or an old one which was deinitialized
after moving it into a new instance.

I think we should assert inside OS(LINUX) that the file descriptor is valid:


  void Semaphore::signal()
  {
  #if OS(LINUX)
      RELEASE_ASSERT_WITH_MESSAGE(m_fd >= 0, "signalled destroyed semaphore");
      // ... rest of the code
  #endif
  }

(or maybe ASSERT_WITH_MESSAGE, and skip the check in release builds)

> Source/WebKit/Platform/IPC/unix/IPCSemaphoreUnix.cpp:116
> +        return false;

Same comment as above regarding this check.

> Source/WebKit/Platform/IPC/unix/IPCSemaphoreUnix.cpp:128
> +        return false;

...and here, too.
Comment 3 Zan Dobersek 2022-04-01 04:36:05 PDT
Created attachment 456346 [details]
Patch

With asserts for valid semaphores.
Comment 4 EWS 2022-04-01 10:44:50 PDT
Committed r292225 (249128@main): <https://commits.webkit.org/249128@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 456346 [details].
Comment 5 Radar WebKit Bug Importer 2022-04-01 10:45:19 PDT
<rdar://problem/91170895>