tor-browser

commit 4afd0a907716a68255010e42b894aac3acc5d015
parent adcf089ed11b2a7edf391e4134b79994715b029d
Author: Valentin Gosu <valentin.gosu@gmail.com>
Date:   Thu, 23 Oct 2025 09:19:07 +0000

Bug 1995267 - Workaround for 1993248 - mark PRFileDesc nonblocking to avoid hanging in PollableEvent::Clear r=necko-reviewers,jesup

Even though calling Clear should only be done when we get non-null out_flags for the socket pair
it should still never block. However, when the socket pair is created by PR_CreatePipe the
PRFileDesc don't properly get their `fd->secret->nonblocking = true` set, so PR_Read might
actually block. This is a workaround until bug 1993248 fixes the issue in NSPR

Differential Revision: https://phabricator.services.mozilla.com/D269201

Diffstat:
M
netwerk/base/PollableEvent.cpp
 | 
34
++++++++++++++++++++++++++++++++++
A
netwerk/test/gtest/TestPollableEvent.cpp
 | 
58
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
M
netwerk/test/gtest/moz.build
 | 
1
+

3 files changed, 93 insertions(+), 0 deletions(-)
diff --git a/netwerk/base/PollableEvent.cpp b/netwerk/base/PollableEvent.cpp
@@ -22,6 +22,34 @@
 #  define USEPIPE 1
 #endif
 
+// PRFilePrivate is defined in a private header of NSPR (primpl.h) we can't
+// include. We need this to be able to set nonblocking=true which PR_CreatePipe
+// fails to do. We can remove this after bug 1993248 is fixed.
+// Note that nsLocalFileWin.cpp also redefines this in order to change the
+// appendMode.
+//-----------------------------------------------------------------------------
+typedef enum {
+  _PR_TRI_TRUE = 1,
+  _PR_TRI_FALSE = 0,
+  _PR_TRI_UNKNOWN = -1
+} _PRTriStateBool;
+
+struct _MDFileDesc {
+  PROsfd osfd;
+};
+
+struct PRFilePrivate {
+  int32_t state;
+  bool nonblocking;
+  _PRTriStateBool inheritable;
+  PRFileDesc* next;
+  int lockCount; /*   0: not locked
+                  *  -1: a native lockfile call is in progress
+                  * > 0: # times the file is locked */
+  bool appendMode;
+  _MDFileDesc md;
+};
+
 namespace mozilla {
 namespace net {
 
@@ -157,6 +185,12 @@ PollableEvent::PollableEvent()
     fd = PR_FileDesc2NativeHandle(mWriteFD);
     flags = fcntl(fd, F_GETFL, 0);
     (void)fcntl(fd, F_SETFL, flags | O_NONBLOCK);
+
+    // Bug 1993248 - PR_CreatePipe doesn't set this flag, so we need to do it
+    // manually to ensure read operations are not blocking. This hack can be
+    // removed once this is fixed in NSPR.
+    mReadFD->secret->nonblocking = true;
+    mWriteFD->secret->nonblocking = true;
   } else {
     mReadFD = nullptr;
     mWriteFD = nullptr;
diff --git a/netwerk/test/gtest/TestPollableEvent.cpp b/netwerk/test/gtest/TestPollableEvent.cpp
@@ -0,0 +1,58 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "TestCommon.h"
+#include "gtest/gtest.h"
+
+#include "mozilla/Atomics.h"
+#include "mozilla/gtest/MozAssertions.h"
+#include "nsCOMPtr.h"
+#include "nsISocketTransport.h"
+#include "../../base/nsSocketTransportService2.h"
+#include "../../base/PollableEvent.h"
+#include "nsServiceManagerUtils.h"
+#include "nsThreadUtils.h"
+
+using namespace mozilla;
+using namespace mozilla::net;
+
+// Test for bug 1993248 - PR_CreatePipe doesn't set nonblocking flag
+// This test verifies that PollableEvent::Clear() doesn't block when called
+TEST(TestPollableEvent, ClearDoesNotBlock)
+{
+  nsCOMPtr<nsISocketTransportService> service =
+      do_GetService("@mozilla.org/network/socket-transport-service;1");
+  ASSERT_TRUE(service);
+
+  auto* sts = gSocketTransportService;
+  ASSERT_TRUE(sts);
+
+  NS_DispatchAndSpinEventLoopUntilComplete(
+      "TestPollableEvent::ClearDoesNotBlock"_ns, sts,
+      NS_NewRunnableFunction("TestPollableEvent::ClearDoesNotBlock", [&]() {
+        // Create a PollableEvent
+        PollableEvent event;
+        ASSERT_TRUE(event.Valid());
+
+        // The constructor signals the event, so Clear() should succeed
+        ASSERT_TRUE(event.Clear());
+
+        // Signal the event
+        ASSERT_TRUE(event.Signal());
+
+        // Clear should succeed and not block
+        ASSERT_TRUE(event.Clear());
+
+        // Calling Clear() again without Signal() should also not block
+        // It should return true even when there's no data available
+        ASSERT_TRUE(event.Clear());
+
+        // Signal and clear multiple times to ensure robustness
+        for (int i = 0; i < 10; i++) {
+          ASSERT_TRUE(event.Signal());
+          ASSERT_TRUE(event.Clear());
+        }
+      }));
+}
diff --git a/netwerk/test/gtest/moz.build b/netwerk/test/gtest/moz.build
@@ -30,6 +30,7 @@ UNIFIED_SOURCES += [
     "TestMIMEInputStream.cpp",
     "TestMozURL.cpp",
     "TestPACMan.cpp",
+    "TestPollableEvent.cpp",
     "TestProtocolProxyService.cpp",
     "TestReadStreamToString.cpp",
     "TestServerTimingHeader.cpp",