stability fixes for snow leopard

direct/src/plugin/p3dInstance.cxx

@@ -249,20 +249,50 @@ P3DInstance(P3D_request_ready_func *func,
 P3DInstance::
 ~P3DInstance() {
   assert(_session == NULL);
+  cleanup();
 
+  if (_browser_script_object != NULL) {
+    P3D_OBJECT_DECREF(_browser_script_object);
+    _browser_script_object = NULL;
+  }
+
+  if (_panda_script_object != NULL) {
+    nout << "panda_script_object ref = "
+         << _panda_script_object->_ref_count << "\n";
+    _panda_script_object->set_instance(NULL);
+    P3D_OBJECT_DECREF(_panda_script_object);
+    _panda_script_object = NULL;
+  }
+
+  nout << "Deleting downloads\n";
+  Downloads::iterator di;
+  for (di = _downloads.begin(); di != _downloads.end(); ++di) {
+    P3DDownload *download = (*di).second;
+    p3d_unref_delete(download);
+  }
+  _downloads.clear();
+  nout << "done deleting downloads\n";
+
+  DESTROY_LOCK(_request_lock);
+
+  // TODO: Is it possible for someone to delete an instance while a
+  // download is still running?  Who will crash when this happens?
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: P3DInstance::cleanup
+//       Access: Public
+//  Description: Invalidates the instance and removes any structures
+//               prior to deleting.
+////////////////////////////////////////////////////////////////////
+void P3DInstance::
+cleanup() {
   if (_auth_session != NULL) {
     _auth_session->shutdown(false);
     p3d_unref_delete(_auth_session);
     _auth_session = NULL;
   }
     
-  P3D_OBJECT_XDECREF(_browser_script_object);
-
-  nout << "panda_script_object ref = "
-       << _panda_script_object->_ref_count << "\n";
-  _panda_script_object->set_instance(NULL);
-  P3D_OBJECT_DECREF(_panda_script_object);
-
   for (int i = 0; i < (int)IT_num_image_types; ++i) {
     _image_files[i].cleanup();
   }
@@ -308,18 +338,40 @@ P3DInstance::
   if (_frame_timer != NULL) {
     CFRunLoopTimerInvalidate(_frame_timer);
     CFRelease(_frame_timer);
+    _frame_timer = NULL;
   }
 
   free_swbuffer();
 #endif    
 
-  DESTROY_LOCK(_request_lock);
+  TiXmlDocument *doc = NULL;
+  ACQUIRE_LOCK(_request_lock);
+  RawRequests::iterator ri;
+  for (ri = _raw_requests.begin(); ri != _raw_requests.end(); ++ri) {
+    doc = (*ri);
+    delete doc;
+  }
+  _raw_requests.clear();
+  RELEASE_LOCK(_request_lock);
 
-  // TODO: empty _raw_requests and _baked_requests queues, and
-  // _downloads map.
+  nout << this << " cleanup baked_requests\n";
+  BakedRequests::iterator bi;
+  for (bi = _baked_requests.begin(); bi != _baked_requests.end(); ++bi) {
+    P3D_request *request = (*bi);
+    nout << this << " cleanup in request " << request << " with " << request->_instance
+         << "\n";
+    finish_request(request, false);
+  }
+  _baked_requests.clear();
+  nout << this << " done cleanup baked_requests\n";
 
-  // TODO: Is it possible for someone to delete an instance while a
-  // download is still running?  Who will crash when this happens?
+  Downloads::iterator di;
+  for (di = _downloads.begin(); di != _downloads.end(); ++di) {
+    P3DDownload *download = (*di).second;
+    download->cancel();
+  }
+
+  _failed = true;
 }
 
 
@@ -750,7 +802,9 @@ add_raw_request(TiXmlDocument *doc) {
 ////////////////////////////////////////////////////////////////////
 void P3DInstance::
 add_baked_request(P3D_request *request) {
+  assert(request->_instance == NULL);
   request->_instance = this;
+  ref();
 
   _baked_requests.push_back(request);
   _request_pending = true;
@@ -762,7 +816,7 @@ add_baked_request(P3D_request *request) {
 
 ////////////////////////////////////////////////////////////////////
 //     Function: P3DInstance::finish_request
-//       Access: Public
+//       Access: Public, Static
 //  Description: Deallocates a previously-returned request from
 //               get_request().  If handled is true, the request has
 //               been handled by the host; otherwise, it has been
@@ -771,6 +825,13 @@ add_baked_request(P3D_request *request) {
 void P3DInstance::
 finish_request(P3D_request *request, bool handled) {
   assert(request != NULL);
+  if (request->_instance == NULL) {
+    nout << "Ignoring empty request " << request << "\n";
+    return;
+  }
+
+  P3DInstanceManager *inst_mgr = P3DInstanceManager::get_global_ptr();
+  assert(inst_mgr->validate_instance(request->_instance) != NULL);
 
   switch (request->_request_type) {
   case P3D_RT_stop:
@@ -778,13 +839,17 @@ finish_request(P3D_request *request, bool handled) {
 
   case P3D_RT_get_url:
     free((char *)request->_request._get_url._url);
+    request->_request._get_url._url = NULL;
     break;
 
   case P3D_RT_notify:
     free((char *)request->_request._notify._message);
+    request->_request._notify._message = NULL;
     break;
   }
 
+  p3d_unref_delete(((P3DInstance *)request->_instance));
+  request->_instance = NULL;
   delete request;
 }
 
@@ -931,6 +996,37 @@ add_package(P3DPackage *package) {
   package->add_instance(this);
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: P3DInstance::remove_package
+//       Access: Public
+//  Description: Indicates that the given package is destructing and
+//               this instance should no longer retain a pointer to
+//               it.  This is normally called only by the P3DPackage
+//               destructor, and it invalidates the instance.
+////////////////////////////////////////////////////////////////////
+void P3DInstance::
+remove_package(P3DPackage *package) {
+  Packages::iterator pi = find(_packages.begin(), _packages.end(), package);
+  if (pi != _packages.end()) {
+    _packages.erase(pi);
+  }
+  pi = find(_downloading_packages.begin(), _downloading_packages.end(), package);
+  if (pi != _downloading_packages.end()) {
+    _downloading_packages.erase(pi);
+  }
+  if (package == _image_package) {
+    _image_package = NULL;
+  }
+  if (package == _certlist_package) {
+    _certlist_package = NULL;
+  }
+  if (package == _p3dcert_package) {
+    _p3dcert_package = NULL;
+  }
+
+  set_failed();
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: P3DInstance::get_packages_info_ready
 //       Access: Public
@@ -1037,6 +1133,7 @@ start_download(P3DDownload *download, bool add_request) {
   // add_request is true in order to ask the plugin for the stream.
   if (add_request) {
     P3D_request *request = new P3D_request;
+    request->_instance = NULL;
     request->_request_type = P3D_RT_get_url;
     request->_request._get_url._url = strdup(download->get_url().c_str());
     request->_request._get_url._unique_id = download_id;
@@ -1098,6 +1195,7 @@ request_stop_main_thread() {
   if (add_request) {
     _requested_stop = true;
     P3D_request *request = new P3D_request;
+    request->_instance = NULL;
     request->_request_type = P3D_RT_stop;
     add_baked_request(request);
   }
@@ -1113,6 +1211,7 @@ request_stop_main_thread() {
 void P3DInstance::
 request_refresh() {
   P3D_request *request = new P3D_request;
+  request->_instance = NULL;
   request->_request_type = P3D_RT_refresh;
   add_baked_request(request);
 }
@@ -2032,6 +2131,7 @@ make_p3d_request(TiXmlElement *xrequest) {
       if (message != NULL) {
         // A notify message from Python code.
         request = new P3D_request;
+        request->_instance = NULL;
         request->_request_type = P3D_RT_notify;
         request->_request._notify._message = strdup(message);
         handle_notify_request(message);
@@ -2080,6 +2180,7 @@ make_p3d_request(TiXmlElement *xrequest) {
     } else if (strcmp(rtype, "stop") == 0) {
       // A stop request from Python code.  This is kind of weird, but OK.
       request = new P3D_request;
+      request->_instance = NULL;
       request->_request_type = P3D_RT_stop;
 
     } else {
@@ -2088,7 +2189,9 @@ make_p3d_request(TiXmlElement *xrequest) {
   }
 
   if (request != NULL) {
+    assert(request->_instance == NULL);
     request->_instance = this;
+    ref();
   }
   return request;
 }
@@ -2164,6 +2267,7 @@ handle_notify_request(const string &message) {
 #ifdef __APPLE__
     // Start a timer to update the frame repeatedly.  This seems to be
     // steadier than waiting for nullEvent.
+    if (_frame_timer == NULL) {
       CFRunLoopTimerContext timer_context;
       memset(&timer_context, 0, sizeof(timer_context));
       timer_context.info = this;
@@ -2171,6 +2275,7 @@ handle_notify_request(const string &message) {
         (NULL, 0, 1.0 / 60.0, 0, 0, timer_callback, &timer_context);
       CFRunLoopRef run_loop = CFRunLoopGetCurrent();
       CFRunLoopAddTimer(run_loop, _frame_timer, kCFRunLoopCommonModes);
+    }
 #endif  // __APPLE__
 
   } else if (message == "onwindowdetach") {
@@ -3400,6 +3505,7 @@ add_cocoa_modifier_flags(unsigned int &swb_flags, int modifiers) {
 void P3DInstance::
 send_notify(const string &message) {
   P3D_request *request = new P3D_request;
+  request->_instance = NULL;
   request->_request_type = P3D_RT_notify;
   request->_request._notify._message = strdup(message.c_str());
   add_baked_request(request);

direct/src/plugin/p3dInstance.h

@@ -56,6 +56,7 @@ public:
               const P3D_token tokens[], size_t num_tokens, 
               int argc, const char *argv[], void *user_data);
   ~P3DInstance();
+  void cleanup();
 
   void set_p3d_url(const string &p3d_url);
   void set_p3d_filename(const string &p3d_filename, const int &p3d_offset = 0);
@@ -73,7 +74,7 @@ public:
   void bake_requests();
   void add_raw_request(TiXmlDocument *doc);
   void add_baked_request(P3D_request *request);
-  void finish_request(P3D_request *request, bool handled);
+  static void finish_request(P3D_request *request, bool handled);
 
   bool feed_url_stream(int unique_id,
                        P3D_result_code result_code,
@@ -94,6 +95,7 @@ public:
 
   void add_package(const string &name, const string &version, P3DHost *host);
   void add_package(P3DPackage *package);
+  void remove_package(P3DPackage *package);
   bool get_packages_info_ready() const;
   bool get_packages_ready() const;
   bool get_packages_failed() const;

direct/src/plugin/p3dInstanceManager.cxx

@@ -418,6 +418,7 @@ start_instance(P3DInstance *inst) {
 ////////////////////////////////////////////////////////////////////
 void P3DInstanceManager::
 finish_instance(P3DInstance *inst) {
+  nout << "finish_instance\n";
   Instances::iterator ii;
   ii = _instances.find(inst);
   assert(ii != _instances.end());
@@ -437,7 +438,10 @@ finish_instance(P3DInstance *inst) {
     }
   }
 
+  inst->cleanup();
+  nout << "done cleanup, calling delete\n";
   p3d_unref_delete(inst);
+  nout << "done finish_instance\n";
 }
 
 ////////////////////////////////////////////////////////////////////

direct/src/plugin/p3dPackage.cxx

@@ -83,6 +83,13 @@ P3DPackage::
   // Tell any pending callbacks that we're no good any more.
   report_done(false);
 
+  // Ditto the outstanding instances.
+  Instances::iterator ii;
+  for (ii = _instances.begin(); ii != _instances.end(); ++ii) {
+    (*ii)->remove_package(this);
+  }
+  _instances.clear();
+
   if (_xconfig != NULL) {
     delete _xconfig;
     _xconfig = NULL;
@@ -102,8 +109,6 @@ P3DPackage::
     delete _temp_contents_file;
     _temp_contents_file = NULL;
   }
-
-  assert(_instances.empty());
 }
 
 ////////////////////////////////////////////////////////////////////

direct/src/plugin/p3d_plugin.cxx

@@ -532,11 +532,7 @@ P3D_request_finish(P3D_request *request, bool handled) {
   assert(P3DInstanceManager::get_global_ptr()->is_initialized());
   ACQUIRE_LOCK(_api_lock);
   if (request != (P3D_request *)NULL) {
-    P3DInstanceManager *inst_mgr = P3DInstanceManager::get_global_ptr();
-    P3DInstance *inst = inst_mgr->validate_instance(request->_instance);
-    if (inst != NULL) {
-      inst->finish_request(request, handled);
-    }
+    P3DInstance::finish_request(request, handled);
   }
   RELEASE_LOCK(_api_lock);
 }

direct/src/plugin_activex/PPInstance.cpp

@@ -591,7 +591,7 @@ std::string PPInstance::GetP3DFilename( )
 
 void PPInstance::HandleRequestLoop() 
 {
-    P3D_instance *p3d_inst = P3D_check_request(false);
+    P3D_instance *p3d_inst = P3D_check_request(0.0);
     while ( p3d_inst != NULL ) 
     {
         P3D_request *request = P3D_instance_get_request(p3d_inst);
@@ -607,7 +607,7 @@ void PPInstance::HandleRequestLoop()
                 nout << "Error handling P3D request. Instance's user data is not a Control \n";
             }
         }
-        p3d_inst = P3D_check_request( false );
+        p3d_inst = P3D_check_request(0.0);
     }
 }
 

direct/src/plugin_npapi/ppInstance.cxx

@@ -603,10 +603,10 @@ stream_as_file(NPStream *stream, const char *fname) {
 ////////////////////////////////////////////////////////////////////
 void PPInstance::
 handle_request(P3D_request *request) {
-  assert(request->_instance == _p3d_inst);
-  if (_failed) {
+  if (_p3d_inst == NULL || _failed) {
     return;
   }
+  assert(request->_instance == _p3d_inst);
 
   bool handled = false;
 
@@ -992,11 +992,6 @@ request_ready(P3D_instance *instance) {
   PPInstance *inst = (PPInstance *)(instance->_user_data);
   assert(inst != NULL);
 
-  {
-    static int n = 0;
-    nout << "request_ready " << ++n << "\n";
-  }
-
   if (has_plugin_thread_async_call) {
 #ifdef HAS_PLUGIN_THREAD_ASYNC_CALL
     // Since we are running at least Gecko 1.9, and we have this very
@@ -1639,15 +1634,20 @@ handle_request_loop() {
     return;
   }
 
-  P3D_instance *p3d_inst = P3D_check_request(false);
+  P3D_instance *p3d_inst = P3D_check_request(0.0);
   while (p3d_inst != (P3D_instance *)NULL) {
     P3D_request *request = P3D_instance_get_request(p3d_inst);
     if (request != (P3D_request *)NULL) {
       PPInstance *inst = (PPInstance *)(p3d_inst->_user_data);
       assert(inst != NULL);
       inst->handle_request(request);
+      if (!is_plugin_loaded()) {
+        // Oops, we may have unloaded the plugin as an indirect effect
+        // of handling the request.  If so, get out of here.
+        return;
       }
-    p3d_inst = P3D_check_request(false);
+    }
+    p3d_inst = P3D_check_request(0.0);
   }
 
   // Also check to see if we have any file_data objects that have