Commit c01652d5 authored by Nika Layzell's avatar Nika Layzell
Browse files

Bug 1834640 - Part 2: Properly shutdown nsSocketTransportService during... 

Bug 1834640 - Part 2: Properly shutdown nsSocketTransportService during xpcom-shutdown-threads, r=jesup,necko-reviewers

This also cleans up some of the unnecessary flags & memebrs which were held
during the changes in bug 1818998. mSelf was unncessary as the runnable
reference will be keeping the instance alive for the same lifetime, and
mSocketThreadShutDown was redundant with !mInitialized.

This change will also fix a potential issue where the socket thread was not
being shut down when switching the browser into an offline state from
`profile-change-net-teardown`, which could've lead to thread leaks when the
browser is returned to an online state. The restore codepath, however, has
been dead for a long time so this is unlikely to be a real issue in practice.

Depends on D178867

Differential Revision: https://phabricator.services.mozilla.com/D178868
parent 12fd3358

netwerk/base/nsSocketTransportService2.cpp

+17 −25
Original line number Diff line number Diff line
@@ -253,7 +253,6 @@ bool nsSocketTransportService::UpdatePortRemapPreference( 
nsSocketTransportService::~nsSocketTransportService() {

  NS_ASSERTION(NS_IsMainThread(), "wrong thread");

  NS_ASSERTION(!mInitialized, "not shutdown properly");

  MOZ_ASSERT(mSocketThreadShutDown);



  gSocketTransportService = nullptr;

}
@@ -414,7 +413,7 @@ bool nsSocketTransportService::CanAttachSocket() { 
    reported900FDLimit = true;

    Telemetry::Accumulate(Telemetry::NETWORK_SESSION_AT_900FD, true);

  }

  MOZ_ASSERT(!mSocketThreadShutDown);

  MOZ_ASSERT(mInitialized);

  return rv;

}
@@ -753,8 +752,6 @@ nsSocketTransportService::Init() { 
    nsresult rv = NS_NewNamedThread("Socket Thread", getter_AddRefs(thread),

                                    this, {.stackSize = GetThreadStackSize()});

    NS_ENSURE_SUCCESS(rv, rv);

    // Since ::Run() will assume it can access this->*, hold a ref for it

    mSelf = this;

  } else {

    // In the child process, we just want a regular nsThread with no socket

    // polling. So we don't want to run the nsSocketTransportService runnable on
@@ -819,10 +816,7 @@ nsSocketTransportService::Shutdown(bool aXpcomShutdown) { 
      observer->Observe();

    }

  }

  if (!XRE_IsContentProcess() ||

      StaticPrefs::network_allow_raw_sockets_in_content_processes_AtStartup()) {

    // signal the socket thread to shutdown, it will do a runnable

    // back to MainThread to call ShutdownThread()



  mShuttingDown = true;



  {
@@ -832,10 +826,12 @@ nsSocketTransportService::Shutdown(bool aXpcomShutdown) { 
      mPollableEvent->Signal();

    }

  }

  } else {

    // Not running the 'real' socket thread, just shut it down.



  // If we're shutting down due to going offline (rather than due to XPCOM

  // shutdown), also tear down the thread. The thread will be shutdown during

  // xpcom-shutdown-threads if during xpcom-shutdown proper.

  if (!aXpcomShutdown) {

    ShutdownThread();

    mSocketThreadShutDown = true;

  }



  return NS_OK;
@@ -1257,10 +1253,6 @@ nsSocketTransportService::Run() { 
  MOZ_ASSERT(mPollList.Length() == 1);

  MOZ_ASSERT(mActiveList.IsEmpty());

  MOZ_ASSERT(mIdleList.IsEmpty());

  mSocketThreadShutDown = true;

  NS_DispatchToMainThread(NS_NewRunnableFunction(

      "nsSocketTransportService::mSelf",

      [self = RefPtr{mSelf.forget()}]() { self->ShutdownThread(); }));



  return NS_OK;

}
@@ -1625,7 +1617,7 @@ nsSocketTransportService::Observe(nsISupports* subject, const char* topic, 
                              nsITimer::TYPE_ONE_SHOT);

    }

  } else if (!strcmp(topic, "xpcom-shutdown-threads")) {

    Shutdown(true);

    ShutdownThread();

  } else if (!strcmp(topic, NS_NETWORK_LINK_TOPIC)) {

    mLastNetworkLinkChangeTime = PR_IntervalNow();

  }

netwerk/base/nsSocketTransportService2.h

+0 −3
Original line number Diff line number Diff line
@@ -160,9 +160,6 @@ class nsSocketTransportService final : public nsPISocketTransportService, 
  Atomic<bool> mInitialized{false};

  // indicates whether we are currently in the process of shutting down

  Atomic<bool> mShuttingDown{false};

  Atomic<bool> mSocketThreadShutDown{false};

  // Effectively owned by the SocketThread

  RefPtr<nsSocketTransportService> mSelf;



  Mutex mLock{"nsSocketTransportService::mLock"};

  // Variables in the next section protected by mLock