From 708d5e37fbae25c03926cba221a3bdb0bfa9e7f9 Mon Sep 17 00:00:00 2001 From: David Rose Date: Tue, 22 Dec 2009 21:15:14 +0000 Subject: [PATCH] stability fixes for snow leopard --- direct/src/plugin/p3dInstance.cxx | 148 +++++++++++++++++++---- direct/src/plugin/p3dInstance.h | 4 +- direct/src/plugin/p3dInstanceManager.cxx | 4 + direct/src/plugin/p3dPackage.cxx | 9 +- direct/src/plugin/p3d_plugin.cxx | 6 +- direct/src/plugin_activex/PPInstance.cpp | 4 +- direct/src/plugin_npapi/ppInstance.cxx | 18 +-- direct/src/plugin_npapi/startup.cxx | 2 +- 8 files changed, 154 insertions(+), 41 deletions(-) diff --git a/direct/src/plugin/p3dInstance.cxx b/direct/src/plugin/p3dInstance.cxx index 8736d40563..377768d703 100644 --- a/direct/src/plugin/p3dInstance.cxx +++ b/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,13 +2267,15 @@ 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. - CFRunLoopTimerContext timer_context; - memset(&timer_context, 0, sizeof(timer_context)); - timer_context.info = this; - _frame_timer = CFRunLoopTimerCreate - (NULL, 0, 1.0 / 60.0, 0, 0, timer_callback, &timer_context); - CFRunLoopRef run_loop = CFRunLoopGetCurrent(); - CFRunLoopAddTimer(run_loop, _frame_timer, kCFRunLoopCommonModes); + if (_frame_timer == NULL) { + CFRunLoopTimerContext timer_context; + memset(&timer_context, 0, sizeof(timer_context)); + timer_context.info = this; + _frame_timer = CFRunLoopTimerCreate + (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); diff --git a/direct/src/plugin/p3dInstance.h b/direct/src/plugin/p3dInstance.h index 8c30792abc..503dc6bf0b 100644 --- a/direct/src/plugin/p3dInstance.h +++ b/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; diff --git a/direct/src/plugin/p3dInstanceManager.cxx b/direct/src/plugin/p3dInstanceManager.cxx index 7895abbbbc..5bfaa6d97f 100644 --- a/direct/src/plugin/p3dInstanceManager.cxx +++ b/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"; } //////////////////////////////////////////////////////////////////// diff --git a/direct/src/plugin/p3dPackage.cxx b/direct/src/plugin/p3dPackage.cxx index 9dc1e2cfb3..9b993e0672 100755 --- a/direct/src/plugin/p3dPackage.cxx +++ b/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()); } //////////////////////////////////////////////////////////////////// diff --git a/direct/src/plugin/p3d_plugin.cxx b/direct/src/plugin/p3d_plugin.cxx index 414514ae9d..f2c09d5dc4 100644 --- a/direct/src/plugin/p3d_plugin.cxx +++ b/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); } diff --git a/direct/src/plugin_activex/PPInstance.cpp b/direct/src/plugin_activex/PPInstance.cpp index c96399eca5..06e0a565d3 100644 --- a/direct/src/plugin_activex/PPInstance.cpp +++ b/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); } } diff --git a/direct/src/plugin_npapi/ppInstance.cxx b/direct/src/plugin_npapi/ppInstance.cxx index 58ccee4c8f..eaab0bcde7 100644 --- a/direct/src/plugin_npapi/ppInstance.cxx +++ b/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 diff --git a/direct/src/plugin_npapi/startup.cxx b/direct/src/plugin_npapi/startup.cxx index 18f4dca2c9..7f759a6e28 100644 --- a/direct/src/plugin_npapi/startup.cxx +++ b/direct/src/plugin_npapi/startup.cxx @@ -372,7 +372,7 @@ NPP_New(NPMIMEType pluginType, NPP instance, uint16_t mode, //////////////////////////////////////////////////////////////////// NPError NPP_Destroy(NPP instance, NPSavedData **save) { - nout << "destroy instance " << instance << ", " + nout << "destroy instance " << instance << ", " << (PPInstance *)instance->pdata << "\n"; nout << "save = " << (void *)save << "\n"; // (*save) = NULL;