Firefox wants to pass eventfds, but kqueues are not passable

[valpackett]

Since this landed in Firefox, it started using an eventfd for a shared reference count thing, and it's trying to pass it over a socket every time there's a video frame decoded by hardware (vaapi).. and it fails because our eventfds fail the (fp->f_ops->fo_flags & DFLAG_PASSABLE) check in kern/uipc_usrreq.c and the sendmsg fails with EOPNOTSUPP. :(

[Child 24571, Chrome_ChildThread] WARNING: pipe error: Operation not supported: file /home/greg/src/hg.mozilla.org/mozilla-unified/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 696

I do have an easy workaround for Firefox (just ifdef'ing out refCountFDs.AppendElement(ipc::FileDescriptor(mGlobalRefCountFd))) but this is quite a surprising problem in general / in theory!

/cc @jbeich

[jiixyj]

Thanks for the report! I've been seeing this as well while trying to get the VA-API acceleration for X11 Firefox to work. This is a tough nut to crack in the shim layer. The shim would not only have to pass the fd (which is not possible in FreeBSD for kqueue fd's as you said), but the shim "context" as well. The latter is not easily shim-able. One could add internalization/externalization functions for this that would have to be called manually before fd sending and after fd receiving.

I tried to replace Firefox's usage of eventfd with memfd_create/shm_open and POSIX semaphores. I've managed to get a VAAPI accelerated image to show, but there were many artifacts and eventually my machine locked up. This patch is still buggy I guess -- sem_init should only be called by the sender of the semaphore, not the receiver if I understand this correctly. The receiver just has to call mmap on the passed memfd to get the sem_t *.

I think the way forward would be to replace the eventfd semaphore usage in Firefox with a portable solution, or even to implement eventfds natively in FreeBSD.

diff --git a/widget/gtk/DMABufSurface.cpp b/widget/gtk/DMABufSurface.cpp
index 57ee246a52cd..349786cf03ac 100644
--- a/widget/gtk/DMABufSurface.cpp
+++ b/widget/gtk/DMABufSurface.cpp
@@ -18,7 +18,13 @@
 #include <sys/time.h>
 #include <dlfcn.h>
 #include <sys/mman.h>
-#include <sys/eventfd.h>
+#ifdef __linux__
+#  include <sys/eventfd.h>
+#else
+#  include <sys/types.h>
+#  include <sys/mman.h>
+#  include <semaphore.h>
+#endif
 #include <poll.h>
 
 #include "mozilla/widget/gbm.h"
@@ -71,14 +77,24 @@ bool DMABufSurface::IsGlobalRefSet() const {
   if (!mGlobalRefCountFd) {
     return false;
   }
+#ifdef __linux__
   struct pollfd pfd;
   pfd.fd = mGlobalRefCountFd;
   pfd.events = POLLIN;
   return poll(&pfd, 1, 0) == 1;
+#else
+  if (!mGlobalRefCountSem) {
+    return false;
+  }
+  int val = 0;
+  (void)sem_getvalue(mGlobalRefCountSem, &val);
+  return val != 0;
+#endif
 }
 
 void DMABufSurface::GlobalRefRelease() {
   MOZ_ASSERT(mGlobalRefCountFd);
+#ifdef __linux__
   uint64_t counter;
   if (read(mGlobalRefCountFd, &counter, sizeof(counter)) != sizeof(counter)) {
     // EAGAIN means the refcount is already zero. It happens when we release
@@ -87,35 +103,98 @@ void DMABufSurface::GlobalRefRelease() {
       NS_WARNING("Failed to unref dmabuf global ref count!");
     }
   }
+#else
+  if (mGlobalRefCountSem == nullptr || sem_trywait(mGlobalRefCountSem) < 0) {
+    NS_WARNING("Failed to unref dmabuf global ref count!");
+  }
+#endif
 }
 
 void DMABufSurface::GlobalRefAdd() {
   MOZ_ASSERT(mGlobalRefCountFd);
+#ifdef __linux__
   uint64_t counter = 1;
   if (write(mGlobalRefCountFd, &counter, sizeof(counter)) != sizeof(counter)) {
+#else
+  if (mGlobalRefCountSem == nullptr || sem_post(mGlobalRefCountSem) < 0) {
+#endif
     NS_WARNING("Failed to ref dmabuf global ref count!");
   }
 }
 
 void DMABufSurface::GlobalRefCountCreate() {
   MOZ_ASSERT(!mGlobalRefCountFd);
+#ifdef __linux__
   mGlobalRefCountFd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK | EFD_SEMAPHORE);
+#else
+  mGlobalRefCountFd = memfd_create("", MFD_CLOEXEC);
+#endif
   if (mGlobalRefCountFd < 0) {
-    NS_WARNING("Failed to create dmabuf global ref count!");
-    mGlobalRefCountFd = 0;
-    return;
+    goto fail1;
+  }
+#ifndef __linux__
+  if (ftruncate(mGlobalRefCountFd, sizeof(sem_t)) < 0 ||
+      !GlobalRefCountMmapSem()) {
+    goto fail2;
+  }
+#endif
+  return;
+
+fail2:
+  close(mGlobalRefCountFd);
+fail1:
+  NS_WARNING("Failed to create dmabuf global ref count!");
+  mGlobalRefCountFd = 0;
+}
+
+#ifdef __linux__
+bool DMABufSurface::GlobalRefCountMmapSem() { return true; }
+#else
+bool DMABufSurface::GlobalRefCountMmapSem() {
+  MOZ_ASSERT(!mGlobalRefCountFd);
+  mGlobalRefCountSem = (sem_t*)mmap(NULL, sizeof(sem_t), PROT_READ | PROT_WRITE,
+                                    MAP_SHARED, mGlobalRefCountFd, 0);
+  if (mGlobalRefCountSem == MAP_FAILED) {
+    goto fail1;
+  }
+
+  if (sem_init(mGlobalRefCountSem, 1, 0) < 0) {
+    goto fail2;
   }
+
+  return true;
+
+fail2:
+  (void)munmap(mGlobalRefCountSem, sizeof(sem_t));
+fail1:
+  NS_WARNING("Failed to mmap dmabuf global ref count!");
+  mGlobalRefCountSem = nullptr;
+  return false;
+}
+#endif
+
+#ifdef __linux__
+void DMABufSurface::GlobalRefCountMunmapSem() {}
+#else
+void DMABufSurface::GlobalRefCountMunmapSem() {
+  if (mGlobalRefCountSem != nullptr) {
+    (void)munmap(mGlobalRefCountSem, sizeof(sem_t));
+  }
+  mGlobalRefCountSem = nullptr;
 }
+#endif
 
 void DMABufSurface::GlobalRefCountImport(int aFd) {
   MOZ_ASSERT(!mGlobalRefCountFd);
   mGlobalRefCountFd = aFd;
+  (void)GlobalRefCountMmapSem();
   GlobalRefAdd();
 }
 
 void DMABufSurface::GlobalRefCountDelete() {
   if (mGlobalRefCountFd) {
     GlobalRefRelease();
+    GlobalRefCountMunmapSem();
     close(mGlobalRefCountFd);
     mGlobalRefCountFd = 0;
   }
@@ -150,6 +229,7 @@ DMABufSurface::DMABufSurface(SurfaceType aSurfaceType)
       mSyncFd(-1),
       mSync(0),
       mGlobalRefCountFd(0),
+      mGlobalRefCountSem(nullptr),
       mUID(gNewSurfaceUID++) {
   for (auto& slot : mDmabufFds) {
     slot = -1;
diff --git a/widget/gtk/DMABufSurface.h b/widget/gtk/DMABufSurface.h
index 854377d3479f..79125bec6c12 100644
--- a/widget/gtk/DMABufSurface.h
+++ b/widget/gtk/DMABufSurface.h
@@ -8,6 +8,7 @@
 #define DMABufSurface_h__
 
 #include <stdint.h>
+#include <semaphore.h>
 #include "GLContext.h"
 #include "GLContextTypes.h"
 #include "mozilla/widget/nsWaylandDisplay.h"
@@ -131,6 +132,8 @@ class DMABufSurface {
   virtual bool Create(const mozilla::layers::SurfaceDescriptor& aDesc) = 0;
   bool FenceImportFromFd();
 
+  bool GlobalRefCountMmapSem();
+  void GlobalRefCountMunmapSem();
   void GlobalRefCountImport(int aFd);
   void GlobalRefCountDelete();
 
@@ -160,6 +163,7 @@ class DMABufSurface {
   RefPtr<mozilla::gl::GLContext> mGL;
 
   int mGlobalRefCountFd;
+  sem_t *mGlobalRefCountSem;
   uint32_t mUID;
 };
 
diff --git a/widget/gtk/moz.build b/widget/gtk/moz.build
index 5c339d496f85..05bc6ee2083e 100644
--- a/widget/gtk/moz.build
+++ b/widget/gtk/moz.build
@@ -173,3 +173,5 @@ if CONFIG['MOZ_ENABLE_DBUS']:
     CXXFLAGS += CONFIG['MOZ_DBUS_GLIB_CFLAGS']
 
 CXXFLAGS += ['-Wno-error=shadow']
+
+OS_LIBS += ['epoll-shim']

[valpackett]

implement eventfds natively in FreeBSD

Yeah, we even have them (and timerfds) in the linuxulator now, so mostly a matter of exposing them natively.

I guess when we have it, linking to epoll-shim would result in the implementation here overriding the libc symbol? There should be some way to prefer native implementations.. e.g. not building one of the *fds when it's already present in the system.

I've managed to get a VAAPI accelerated image to show, but there were many artifacts and eventually my machine locked up

Possibly unrelated? The ref count was not there in the beginning, and it's still kinda optional — everything works again for me (on wayland) after just commenting out the passing of the refcount (refCountFDs.AppendElement(ipc::FileDescriptor(mGlobalRefCountFd)))

[valpackett]

Here's a patch for native eventfd: https://reviews.freebsd.org/D26668 :)

[jiixyj]

There should be some way to prefer native implementations.. e.g. not building one of the *fds when it's already present in the system.

Sounds good! This shouldn't be too hard to implement. As you said, currently libepoll-shim.so would conflict with a working eventfd implementation from libc.

[jiixyj]

I've added a test and README comment documenting the current behavior.

[jiixyj]

@myfreeweb : Great to see support for native eventfds in FreeBSD!

I've implemented detection/support of native eventfds here: 95a356b For now it lives in a branch. Once the tests pass everywhere, I'll merge it to master.

[jiixyj]

The support for native eventfds is now merged to master and available in FreeBSD ports. Closing this issue. Thanks again!