Bug 1363653 - Make AltDataOutputStreamParent not send any messages after object destruction r=mayhemer draft
authorValentin Gosu <valentin.gosu@gmail.com>
Thu, 11 May 2017 21:35:37 +0200
changeset 576501 8f918d24595248149ebd3857e05e38dc5237059b
parent 574065 1fda52a1f3b81cf1a821155998dca637bb64e3d9
child 628211 b7aeb05dc0e3c8358467d3369dff5d1e0b2c57fc
push id58377
push uservalentin.gosu@gmail.com
push dateThu, 11 May 2017 19:36:02 +0000
reviewersmayhemer
bugs1363653
milestone55.0a1
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