Bug 238596

Created attachment 456214 [details]
Patch

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.

Created attachment 456346 [details]
Patch

With asserts for valid semaphores.

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].

<rdar://problem/91170895>