From 7700e8dfaa9e42e59033dc9eca4ea288006b79ea Mon Sep 17 00:00:00 2001
From: Junior Hsu <juhsu@mozilla.com>
Date: Tue, 29 Oct 2019 21:01:31 +0000
Subject: [PATCH] Bug 1580288, r=mayhemer

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

--HG--
extra : moz-landing-system : lando
---
 netwerk/protocol/http/nsHttpTransaction.cpp | 29 +++++++++++++++------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/netwerk/protocol/http/nsHttpTransaction.cpp b/netwerk/protocol/http/nsHttpTransaction.cpp
index 4467e3a2101d..ff2b9ade1687 100644
--- a/netwerk/protocol/http/nsHttpTransaction.cpp
+++ b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -25,6 +25,7 @@
 #include "nsIPipe.h"
 #include "nsCRT.h"
 #include "mozilla/Tokenizer.h"
+#include "mozilla/Move.h"
 #include "TCPFastOpenLayer.h"
 
 #include "nsISeekableStream.h"
@@ -2368,8 +2369,17 @@ void nsHttpTransaction::Refused0RTT() {
 void nsHttpTransaction::SetHttpTrailers(nsCString& aTrailers) {
   LOG(("nsHttpTransaction::SetHttpTrailers %p", this));
   LOG(("[\n    %s\n]", aTrailers.BeginReading()));
-  if (!mForTakeResponseTrailers) {
-    mForTakeResponseTrailers = new nsHttpHeaderArray();
+
+  // Introduce a local variable to minimize the critical section.
+  nsAutoPtr<nsHttpHeaderArray> httpTrailers(new nsHttpHeaderArray());
+  // Given it's usually null, use double-check locking for performance.
+  if (mForTakeResponseTrailers) {
+    MutexAutoLock lock(*nsHttp::GetLock());
+    if (mForTakeResponseTrailers) {
+      // Copy the trailer. |TakeResponseTrailers| gets the original trailer
+      // until the final swap.
+      *httpTrailers = *mForTakeResponseTrailers;
+    }
   }
 
   int32_t cur = 0;
@@ -2386,21 +2396,24 @@ void nsHttpTransaction::SetHttpTrailers(nsCString& aTrailers) {
     nsHttpAtom hdr = {nullptr};
     nsAutoCString hdrNameOriginal;
     nsAutoCString val;
-    if (NS_SUCCEEDED(mForTakeResponseTrailers->ParseHeaderLine(
-            line, &hdr, &hdrNameOriginal, &val))) {
+    if (NS_SUCCEEDED(httpTrailers->ParseHeaderLine(line, &hdr, &hdrNameOriginal,
+                                                   &val))) {
       if (hdr == nsHttp::Server_Timing) {
-        Unused << mForTakeResponseTrailers->SetHeaderFromNet(
-            hdr, hdrNameOriginal, val, true);
+        Unused << httpTrailers->SetHeaderFromNet(hdr, hdrNameOriginal, val,
+                                                 true);
       }
     }
 
     cur = newline + 1;
   }
 
-  if (mForTakeResponseTrailers->Count() == 0) {
+  if (httpTrailers->Count() == 0) {
     // Didn't find a Server-Timing header, so get rid of this.
-    mForTakeResponseTrailers = nullptr;
+    httpTrailers = nullptr;
   }
+
+  MutexAutoLock lock(*nsHttp::GetLock());
+  Swap(mForTakeResponseTrailers, httpTrailers);
 }
 
 bool nsHttpTransaction::IsWebsocketUpgrade() {