kenz-gelsoft / gecko-dev

Read-only Git mirror of the Mercurial gecko repositories at https://hg.mozilla.org. How to contribute: https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html
https://firefox-source-docs.mozilla.org/setup/index.html
Other
12 stars 1 forks source link

Investigate IPDLTest_CrossProcess.TestManyHandles failure #48

Open kenz-gelsoft opened 1 week ago

kenz-gelsoft commented 1 week ago

may be related to small chunk communication in IPC

kenz-gelsoft commented 1 week ago

Haiku supports only 32 FDs in the same process as waddlesplash told me https://github.com/kenz-gelsoft/gecko-dev/issues/34#issuecomment-2297569680

diff --git a/ipc/ipdl/test/gtest/TestManyHandles.cpp b/ipc/ipdl/test/gtest/TestManyHandles.cpp
index ead8af22b1f9..8911cbb7ce6e 100644
--- a/ipc/ipdl/test/gtest/TestManyHandles.cpp
+++ b/ipc/ipdl/test/gtest/TestManyHandles.cpp
@@ -19,7 +19,7 @@ class TestManyHandlesChild : public PTestManyHandlesChild {

  public:
   IPCResult RecvManyHandles(nsTArray<FileDescriptor>&& aDescrs) override {
-    EXPECT_EQ(aDescrs.Length(), 500u);
+    EXPECT_EQ(aDescrs.Length(), 32u);
     for (int i = 0; i < static_cast<int>(aDescrs.Length()); ++i) {
       UniqueFileHandle handle = aDescrs[i].TakePlatformHandle();
       int value;
@@ -51,7 +51,7 @@ class TestManyHandlesParent : public PTestManyHandlesParent {

 IPDL_TEST(TestManyHandles) {
   nsTArray<FileDescriptor> descrs;
-  for (int i = 0; i < 500; ++i) {
+  for (int i = 0; i < 32; ++i) {
     const int size = sizeof(i);
     UniqueFileHandle readPipe;
     UniqueFileHandle writePipe;
kenz-gelsoft commented 1 week ago

searched around 500 limit or maxfd max_fd like defs, following two definitions found

patched like this, but early tab crash didn't fix

diff --git a/ipc/chromium/src/chrome/common/ipc_channel_posix.h b/ipc/chromium/src/chrome/common/ipc_channel_posix.h
index b70640d04e12..79064247e800 100644
--- a/ipc/chromium/src/chrome/common/ipc_channel_posix.h
+++ b/ipc/chromium/src/chrome/common/ipc_channel_posix.h
@@ -153,7 +153,11 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher {
   // here. Consequently, we pick a number here that is at least CMSG_SPACE(0) on
   // all platforms. We assert at runtime, in Channel::ChannelImpl::Init, that
   // it's big enough.
+#ifdef XP_HAIKU
+  static constexpr size_t kControlBufferMaxFds = 32;
+#else
   static constexpr size_t kControlBufferMaxFds = 200;
+#endif
   static constexpr size_t kControlBufferHeaderSize = 32;
   static constexpr size_t kControlBufferSize =
       kControlBufferMaxFds * sizeof(int) + kControlBufferHeaderSize;
diff --git a/ipc/chromium/src/base/process_util_posix.cc b/ipc/chromium/src/base/process_util_posix.cc
index a33b94c74d55..5dcb1c6ad838 100644
--- a/ipc/chromium/src/base/process_util_posix.cc
+++ b/ipc/chromium/src/base/process_util_posix.cc
@@ -123,12 +123,15 @@ void CloseSuperfluousFds(void* aCtx, bool (*aShouldPreserve)(void*, int)) {
 #if defined(ANDROID)
   static const rlim_t kSystemDefaultMaxFds = 1024;
   static const char kFDDir[] = "/proc/self/fd";
-#elif defined(XP_LINUX) || defined(XP_SOLARIS) || defined(XP_HAIKU)
+#elif defined(XP_LINUX) || defined(XP_SOLARIS)
   static const rlim_t kSystemDefaultMaxFds = 8192;
   static const char kFDDir[] = "/proc/self/fd";
 #elif defined(XP_DARWIN)
   static const rlim_t kSystemDefaultMaxFds = 256;
   static const char kFDDir[] = "/dev/fd";
+#elif defined(XP_HAIKU)
+  static const rlim_t kSystemDefaultMaxFds = 32;
+  static const char kFDDir[] = "/dev/fd";
 #elif defined(__DragonFly__) || defined(XP_FREEBSD) || defined(XP_NETBSD) || \
     defined(XP_OPENBSD)
   // the getrlimit below should never fail, so whatever ..
kenz-gelsoft commented 1 week ago

nsTArray<FileDescriptor> used from IPDL generated codn

https://searchfox.org/mozilla-esr128/source/gfx/layers/ipc/LayersSurfaces.ipdlh#70

kenz-gelsoft commented 1 week ago

If we revert small chunk crash workaround

diff --git a/gfx/layers/wr/IpcResourceUpdateQueue.cpp b/gfx/layers/wr/IpcResourceUpdateQueue.cpp
index dcbd9e3a42be..5c9e7da8302f 100644
--- a/gfx/layers/wr/IpcResourceUpdateQueue.cpp
+++ b/gfx/layers/wr/IpcResourceUpdateQueue.cpp
@@ -51,8 +53,8 @@ layers::OffsetRange ShmSegmentsWriter::Write(Range<uint8_t> aBytes) {
   const size_t start = mCursor;
   const size_t length = aBytes.length();

-//  if (length >= mChunkSize * 4) {
-  if (length > 0) {
+  if (length >= mChunkSize * 4) {
+//  if (length > 0) {
     auto range = AllocLargeChunk(length);
     if (range.length()) {
       // Allocation was successful

It succeeds. So this failure comes from above workaround.

Running GTest tests...
Note: Google Test filter = IPDLTest_CrossProcess.TestManyHandles
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from IPDLTest_CrossProcess
[ RUN      ] IPDLTest_CrossProcess.TestManyHandles
(snip)
[----------] 1 test from IPDLTest_CrossProcess (88 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (88 ms total)
[  PASSED  ] 1 test.
kenz-gelsoft commented 1 week ago