From 32df05b5286d3e798576c4ea2ff026c94dce90fa Mon Sep 17 00:00:00 2001
From: rdb <git@rdb.name>
Date: Tue, 27 Nov 2018 21:16:03 +0100
Subject: [PATCH] Fix crash when unmounting/closing multifile while streams are
 open

It's not really reasonable to expect a user to find every occurrence of a cached resource that might be using an open stream and remove it or crash otherwise.

This is fixed by keeping the multifile stream open as long as any substreams are still pointing to it, using a simplified reference counting (care is taken not to fully make StreamWrapper reference-counted, since it's not in express and existing uses should not be broken).

Fixes #449
Also see #428
---
 dtool/src/prc/streamWrapper.I      | 18 ++++++++++++++++++
 dtool/src/prc/streamWrapper.h      | 10 ++++++++++
 panda/src/express/multifile.cxx    |  5 ++++-
 panda/src/express/subStreamBuf.cxx | 13 +++++++++++++
 panda/src/express/subStreamBuf.h   |  1 +
 5 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/dtool/src/prc/streamWrapper.I b/dtool/src/prc/streamWrapper.I
index 0f9d65d414..6d2578262c 100644
--- a/dtool/src/prc/streamWrapper.I
+++ b/dtool/src/prc/streamWrapper.I
@@ -58,6 +58,24 @@ release() {
   _lock.unlock();
 }
 
+/**
+ * Increments the reference count.  Only has impact if the class that manages
+ * this StreamWrapper's lifetime (eg. Multifile) respects it.
+ */
+INLINE void StreamWrapperBase::
+ref() const {
+  AtomicAdjust::inc(_ref_count);
+}
+
+/**
+ * Decrements the reference count.  Only has impact if the class that manages
+ * this StreamWrapper's lifetime (eg. Multifile) respects it.
+ */
+INLINE bool StreamWrapperBase::
+unref() const {
+  return AtomicAdjust::dec(_ref_count);
+}
+
 /**
  *
  */
diff --git a/dtool/src/prc/streamWrapper.h b/dtool/src/prc/streamWrapper.h
index b5a4d5bf72..a837540129 100644
--- a/dtool/src/prc/streamWrapper.h
+++ b/dtool/src/prc/streamWrapper.h
@@ -16,6 +16,7 @@
 
 #include "dtoolbase.h"
 #include "mutexImpl.h"
+#include "atomicAdjust.h"
 
 /**
  * The base class for both IStreamWrapper and OStreamWrapper, this provides
@@ -30,8 +31,17 @@ PUBLISHED:
   INLINE void acquire();
   INLINE void release();
 
+public:
+  INLINE void ref() const;
+  INLINE bool unref() const;
+
 private:
   MutexImpl _lock;
+
+  // This isn't really designed as a reference counted class, but it is useful
+  // to treat it as one when dealing with substreams created by Multifile.
+  mutable AtomicAdjust::Integer _ref_count = 1;
+
 #ifdef SIMPLE_THREADS
   // In the SIMPLE_THREADS case, we need to use a bool flag, because MutexImpl
   // defines to nothing in this case--but we still need to achieve a form of
diff --git a/panda/src/express/multifile.cxx b/panda/src/express/multifile.cxx
index e625861f87..cdb1c56ec8 100644
--- a/panda/src/express/multifile.cxx
+++ b/panda/src/express/multifile.cxx
@@ -333,7 +333,10 @@ close() {
   if (_owns_stream) {
     // We prefer to delete the IStreamWrapper over the ostream, if possible.
     if (_read != nullptr) {
-      delete _read;
+      // Only delete it if no SubStream is still referencing it.
+      if (!_read->unref()) {
+        delete _read;
+      }
     } else if (_write != nullptr) {
       delete _write;
     }
diff --git a/panda/src/express/subStreamBuf.cxx b/panda/src/express/subStreamBuf.cxx
index 4d773e359f..14a05f1bfb 100644
--- a/panda/src/express/subStreamBuf.cxx
+++ b/panda/src/express/subStreamBuf.cxx
@@ -88,6 +88,13 @@ open(IStreamWrapper *source, OStreamWrapper *dest, streampos start, streampos en
   _append = append;
   _gpos = _start;
   _ppos = _start;
+
+  if (source != nullptr) {
+    source->ref();
+  }
+  if (dest != nullptr) {
+    dest->ref();
+  }
 }
 
 /**
@@ -98,6 +105,12 @@ close() {
   // Make sure the write buffer is flushed.
   sync();
 
+  if (_source != nullptr && !_source->unref()) {
+    delete _source;
+  }
+  if (_dest != nullptr && !_dest->unref()) {
+    delete _dest;
+  }
   _source = nullptr;
   _dest = nullptr;
   _start = 0;
diff --git a/panda/src/express/subStreamBuf.h b/panda/src/express/subStreamBuf.h
index 779f43d94c..1f0c511e51 100644
--- a/panda/src/express/subStreamBuf.h
+++ b/panda/src/express/subStreamBuf.h
@@ -23,6 +23,7 @@
 class EXPCL_PANDA_EXPRESS SubStreamBuf : public std::streambuf {
 public:
   SubStreamBuf();
+  SubStreamBuf(const SubStreamBuf &copy) = delete;
   virtual ~SubStreamBuf();
 
   void open(IStreamWrapper *source, OStreamWrapper *dest, std::streampos start, std::streampos end, bool append);