Bug 1363653 - Make AltDataOutputStreamParent not send any messages after object destruction r=mayhemer
authorValentin Gosu <valentin.gosu@gmail.com>
Thu, 11 May 2017 21:35:37 +0200
changeset 357916 266e09e18a8e7ba8e9da62d819985e484f939dcb
parent 357915 6a82d640aa274f6532528bd45d9e86ede901f44d
child 357917 387d8415d05e7f1dc96ed3adb441c54f232baf0d
push id31807
push usercbook@mozilla.com
push dateFri, 12 May 2017 12:34:43 +0000
treeherdermozilla-central@bcb5e1ff13f8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmayhemer
bugs1363653
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1363653 - Make AltDataOutputStreamParent not send any messages after object destruction r=mayhemer This patch prevents the following error: AltDataOutputStreamChild::Close -> SendClose() AltDataOutputStreamChild::Release -> Send__delete__() AltDataOutputStreamParent::RecvClose() -> SendError() AltDataOutputStreamParent::ActorDestroy -> mIPCOpen = false AltDataOutputStreamChild::RecvError -> === Crash - object was deleted === We introduce the DeleteSelf message. AltDataOutputStreamChild::Release -> SendDeleteSelf() AltDataOutputStreamParent::RecvDeleteSelf -> mIPCOpen = false; SendDeleteSelf() AltDataOutputStreamChild::RecvDeleteSelf -> Send__delete__() The parent will not send any more messages after receiving the DeleteSelf message. MozReview-Commit-ID: I9RQ5I7eSt9
netwerk/protocol/http/AltDataOutputStreamChild.cpp
netwerk/protocol/http/AltDataOutputStreamChild.h
netwerk/protocol/http/AltDataOutputStreamParent.cpp
netwerk/protocol/http/AltDataOutputStreamParent.h
netwerk/protocol/http/PAltDataOutputStream.ipdl
--- a/netwerk/protocol/http/AltDataOutputStreamChild.cpp
+++ b/netwerk/protocol/http/AltDataOutputStreamChild.cpp
@@ -10,20 +10,21 @@ NS_IMPL_ADDREF(AltDataOutputStreamChild)
 NS_IMETHODIMP_(MozExternalRefCountType) AltDataOutputStreamChild::Release()
 {
   NS_PRECONDITION(0 != mRefCnt, "dup release");
   MOZ_ASSERT(NS_IsMainThread(), "Main thread only");
   --mRefCnt;
   NS_LOG_RELEASE(this, mRefCnt, "AltDataOutputStreamChild");
 
   if (mRefCnt == 1 && mIPCOpen) {
-    // Send_delete calls NeckoChild::PAltDataOutputStreamChild, which will release
-    // again to refcount == 0
-    PAltDataOutputStreamChild::Send__delete__(this);
-    return 0;
+    // The only reference left is the IPDL one. After the parent replies back
+    // with a DeleteSelf message, the child will call Send__delete__(this),
+    // decrementing the ref count and triggering the destructor.
+    SendDeleteSelf();
+    return 1;
   }
 
   if (mRefCnt == 0) {
     mRefCnt = 1; /* stabilize */
     delete this;
     return 0;
   }
   return mRefCnt;
@@ -141,11 +142,17 @@ AltDataOutputStreamChild::IsNonBlocking(
 
 mozilla::ipc::IPCResult
 AltDataOutputStreamChild::RecvError(const nsresult& err)
 {
   mError = err;
   return IPC_OK();
 }
 
+mozilla::ipc::IPCResult
+AltDataOutputStreamChild::RecvDeleteSelf()
+{
+  PAltDataOutputStreamChild::Send__delete__(this);
+  return IPC_OK();
+}
 
 } // namespace net
 } // namespace mozilla
--- a/netwerk/protocol/http/AltDataOutputStreamChild.h
+++ b/netwerk/protocol/http/AltDataOutputStreamChild.h
@@ -22,16 +22,17 @@ public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIOUTPUTSTREAM
   explicit AltDataOutputStreamChild();
 
   void AddIPDLReference();
   void ReleaseIPDLReference();
   // Saves an error code which will be reported to the writer on the next call.
   virtual mozilla::ipc::IPCResult RecvError(const nsresult& err) override;
+  virtual mozilla::ipc::IPCResult RecvDeleteSelf() override;
 
 private:
   virtual ~AltDataOutputStreamChild();
   // Sends data to the parent process in 256k chunks.
   bool WriteDataInChunks(const nsCString& data);
 
   bool mIPCOpen;
   // If there was an error opening the output stream or writing to it on the
--- a/netwerk/protocol/http/AltDataOutputStreamParent.cpp
+++ b/netwerk/protocol/http/AltDataOutputStreamParent.cpp
@@ -68,10 +68,18 @@ AltDataOutputStreamParent::RecvClose()
 }
 
 void
 AltDataOutputStreamParent::ActorDestroy(ActorDestroyReason aWhy)
 {
   mIPCOpen = false;
 }
 
+mozilla::ipc::IPCResult
+AltDataOutputStreamParent::RecvDeleteSelf()
+{
+  mIPCOpen = false;
+  Unused << SendDeleteSelf();
+  return IPC_OK();
+}
+
 } // namespace net
 } // namespace mozilla
--- a/netwerk/protocol/http/AltDataOutputStreamParent.h
+++ b/netwerk/protocol/http/AltDataOutputStreamParent.h
@@ -32,16 +32,17 @@ public:
   virtual mozilla::ipc::IPCResult RecvWriteData(const nsCString& data) override;
   // Called when AltDataOutputStreamChild::Close() is
   // Closes and nulls the output stream.
   virtual mozilla::ipc::IPCResult RecvClose() override;
   virtual void ActorDestroy(ActorDestroyReason aWhy) override;
 
   // Sets an error that will be reported to the content process.
   void SetError(nsresult status) { mStatus = status; }
+  virtual mozilla::ipc::IPCResult RecvDeleteSelf() override;
 
 private:
   virtual ~AltDataOutputStreamParent();
   nsCOMPtr<nsIOutputStream> mOutputStream;
   // In case any error occurs mStatus will be != NS_OK, and this status code will
   // be sent to the content process asynchronously.
   nsresult mStatus;
   bool     mIPCOpen;
--- a/netwerk/protocol/http/PAltDataOutputStream.ipdl
+++ b/netwerk/protocol/http/PAltDataOutputStream.ipdl
@@ -14,19 +14,28 @@ protocol PAltDataOutputStream
 {
   manager PNecko;
 
 parent:
   // Sends data from the child to the parent that will be written to the cache.
   async WriteData(nsCString data);
   // Signals that writing to the output stream is done.
   async Close();
+
   async __delete__();
 
 child:
   // The parent calls this method to signal that an error has ocurred.
   // This may mean that opening the output stream has failed or that writing to
   // the stream has returned an error.
   async Error(nsresult err);
+
+both:
+  // After sending this message, the parent will respond by sending DeleteSelf
+  // back to the child, after which it is guaranteed to not send any more IPC
+  // messages.
+  // When receiving this message, the child will send __delete__ tearing down
+  // the IPC channel.
+  async DeleteSelf();
 };
 
 } // namespace net
 } // namespace mozilla