From 6b908cb5256e290334a65c554127f359cf9ae62d Mon Sep 17 00:00:00 2001 From: David Rose Date: Tue, 28 Jul 2009 20:17:14 +0000 Subject: [PATCH] a few race conditions --- direct/src/plugin/p3dInstanceManager.cxx | 18 +++++ direct/src/plugin/p3dInstanceManager.h | 2 + direct/src/plugin/p3dWinSplashWindow.cxx | 85 +++++++++++++++--------- direct/src/plugin/p3dWinSplashWindow.h | 1 + direct/src/plugin/p3d_plugin.cxx | 70 +++++++++++++++---- 5 files changed, 133 insertions(+), 43 deletions(-) diff --git a/direct/src/plugin/p3dInstanceManager.cxx b/direct/src/plugin/p3dInstanceManager.cxx index 19fc21547e..5143fcb921 100644 --- a/direct/src/plugin/p3dInstanceManager.cxx +++ b/direct/src/plugin/p3dInstanceManager.cxx @@ -208,6 +208,24 @@ finish_instance(P3DInstance *inst) { unref_delete(inst); } +//////////////////////////////////////////////////////////////////// +// Function: P3DInstanceManager::validate_instance +// Access: Public +// Description: Returns the P3DInstance pointer corresponding to the +// indicated P3D_instance if it is valid, or NULL if it +// is not. +//////////////////////////////////////////////////////////////////// +P3DInstance *P3DInstanceManager:: +validate_instance(P3D_instance *instance) { + Instances::iterator ii; + ii = _instances.find((P3DInstance *)instance); + if (ii != _instances.end()) { + return (*ii); + } + + return NULL; +} + //////////////////////////////////////////////////////////////////// // Function: P3DInstanceManager::check_request // Access: Public diff --git a/direct/src/plugin/p3dInstanceManager.h b/direct/src/plugin/p3dInstanceManager.h index 5b8aa48ce1..d09cc7f69e 100644 --- a/direct/src/plugin/p3dInstanceManager.h +++ b/direct/src/plugin/p3dInstanceManager.h @@ -52,6 +52,8 @@ public: const P3D_token tokens[], size_t num_tokens); void finish_instance(P3DInstance *inst); + P3DInstance *validate_instance(P3D_instance *instance); + P3DInstance *check_request(); void wait_request(); diff --git a/direct/src/plugin/p3dWinSplashWindow.cxx b/direct/src/plugin/p3dWinSplashWindow.cxx index 192aec7d0b..9341cb3fd6 100755 --- a/direct/src/plugin/p3dWinSplashWindow.cxx +++ b/direct/src/plugin/p3dWinSplashWindow.cxx @@ -41,8 +41,6 @@ P3DWinSplashWindow(P3DInstance *inst) : _install_progress = 0.0; INIT_LOCK(_install_lock); - - start_thread(); } //////////////////////////////////////////////////////////////////// @@ -57,6 +55,22 @@ P3DWinSplashWindow:: DESTROY_LOCK(_install_lock); } +//////////////////////////////////////////////////////////////////// +// Function: P3DWinSplashWindow::set_wparams +// Access: Public, Virtual +// Description: Changes the window parameters, e.g. to resize or +// reposition the window; or sets the parameters for the +// first time, creating the initial window. +//////////////////////////////////////////////////////////////////// +void P3DWinSplashWindow:: +set_wparams(const P3DWindowParams &wparams) { + P3DSplashWindow::set_wparams(wparams); + + if (_thread_id == 0) { + start_thread(); + } +} + //////////////////////////////////////////////////////////////////// // Function: P3DWinSplashWindow::set_image_filename // Access: Public, Virtual @@ -76,13 +90,15 @@ set_image_filename(const string &image_filename, } RELEASE_LOCK(_install_lock); - // Post a silly message to spin the message loop. - PostThreadMessage(_thread_id, WM_USER, 0, 0); + if (_thread_id != 0) { + // Post a silly message to spin the message loop. + PostThreadMessage(_thread_id, WM_USER, 0, 0); - if (!_thread_running && _thread_continue) { - // The user must have closed the window. Let's shut down the - // instance, too. - _inst->request_stop(); + if (!_thread_running && _thread_continue) { + // The user must have closed the window. Let's shut down the + // instance, too. + _inst->request_stop(); + } } } @@ -101,13 +117,15 @@ set_install_label(const string &install_label) { } RELEASE_LOCK(_install_lock); - // Post a silly message to spin the message loop. - PostThreadMessage(_thread_id, WM_USER, 0, 0); + if (_thread_id != 0) { + // Post a silly message to spin the message loop. + PostThreadMessage(_thread_id, WM_USER, 0, 0); - if (!_thread_running && _thread_continue) { - // The user must have closed the window. Let's shut down the - // instance, too. - _inst->request_stop(); + if (!_thread_running && _thread_continue) { + // The user must have closed the window. Let's shut down the + // instance, too. + _inst->request_stop(); + } } } @@ -124,13 +142,15 @@ set_install_progress(double install_progress) { _install_progress = install_progress; RELEASE_LOCK(_install_lock); - // Post a silly message to spin the message loop. - PostThreadMessage(_thread_id, WM_USER, 0, 0); + if (_thread_id != 0) { + // Post a silly message to spin the message loop. + PostThreadMessage(_thread_id, WM_USER, 0, 0); - if (!_thread_running && _thread_continue) { - // The user must have closed the window. Let's shut down the - // instance, too. - _inst->request_stop(); + if (!_thread_running && _thread_continue) { + // The user must have closed the window. Let's shut down the + // instance, too. + _inst->request_stop(); + } } } @@ -198,19 +218,24 @@ start_thread() { void P3DWinSplashWindow:: stop_thread() { _thread_continue = false; - // Post a silly message to spin the message loop. - PostThreadMessage(_thread_id, WM_USER, 0, 0); - // We can't actually wait for the thread to finish, since there - // might be a deadlock there: the thread can't finish deleting its - // window unless we're pumping the message loop for the parent, - // which won't happen if we're sitting here waiting. No worries; we - // don't *really* need to wait for the thread. + if (_thread_id != 0) { + // Post a silly message to spin the message loop. + PostThreadMessage(_thread_id, WM_USER, 0, 0); + } - // WaitForSingleObject(_thread, INFINITE); + if (_thread != NULL){ + // We can't actually wait for the thread to finish, since there + // might be a deadlock there: the thread can't finish deleting its + // window unless we're pumping the message loop for the parent, + // which won't happen if we're sitting here waiting. No worries; + // we don't *really* need to wait for the thread. + + // WaitForSingleObject(_thread, INFINITE); - CloseHandle(_thread); - _thread = NULL; + CloseHandle(_thread); + _thread = NULL; + } } //////////////////////////////////////////////////////////////////// diff --git a/direct/src/plugin/p3dWinSplashWindow.h b/direct/src/plugin/p3dWinSplashWindow.h index 9836650dc4..443024afd6 100755 --- a/direct/src/plugin/p3dWinSplashWindow.h +++ b/direct/src/plugin/p3dWinSplashWindow.h @@ -34,6 +34,7 @@ public: P3DWinSplashWindow(P3DInstance *inst); virtual ~P3DWinSplashWindow(); + virtual void set_wparams(const P3DWindowParams &wparams); virtual void set_image_filename(const string &image_filename, bool image_filename_temp); virtual void set_install_label(const string &install_label); diff --git a/direct/src/plugin/p3d_plugin.cxx b/direct/src/plugin/p3d_plugin.cxx index da616d62f5..49a96c9957 100644 --- a/direct/src/plugin/p3d_plugin.cxx +++ b/direct/src/plugin/p3d_plugin.cxx @@ -91,8 +91,11 @@ P3D_instance_start(P3D_instance *instance, const char *p3d_filename, } ACQUIRE_LOCK(_api_lock); P3DInstanceManager *inst_mgr = P3DInstanceManager::get_global_ptr(); - bool result = inst_mgr->start_instance - ((P3DInstance *)instance, p3d_filename, tokens, num_tokens); + P3DInstance *inst = inst_mgr->validate_instance(instance); + bool result = false; + if (inst != NULL) { + result = inst_mgr->start_instance(inst, p3d_filename, tokens, num_tokens); + } RELEASE_LOCK(_api_lock); return result; } @@ -102,7 +105,10 @@ P3D_instance_finish(P3D_instance *instance) { assert(P3DInstanceManager::get_global_ptr()->is_initialized()); ACQUIRE_LOCK(_api_lock); P3DInstanceManager *inst_mgr = P3DInstanceManager::get_global_ptr(); - inst_mgr->finish_instance((P3DInstance *)instance); + P3DInstance *inst = inst_mgr->validate_instance(instance); + if (inst != NULL) { + inst_mgr->finish_instance(inst); + } RELEASE_LOCK(_api_lock); } @@ -117,7 +123,11 @@ P3D_instance_setup_window(P3D_instance *instance, win_width, win_height, parent_window); ACQUIRE_LOCK(_api_lock); - ((P3DInstance *)instance)->set_wparams(wparams); + P3DInstanceManager *inst_mgr = P3DInstanceManager::get_global_ptr(); + P3DInstance *inst = inst_mgr->validate_instance(instance); + if (inst != NULL) { + inst->set_wparams(wparams); + } RELEASE_LOCK(_api_lock); } @@ -331,7 +341,12 @@ P3D_instance_get_panda_script_object(P3D_instance *instance) { assert(P3DInstanceManager::get_global_ptr()->is_initialized()); ACQUIRE_LOCK(_api_lock); - P3D_object *result = ((P3DInstance *)instance)->get_panda_script_object(); + P3DInstanceManager *inst_mgr = P3DInstanceManager::get_global_ptr(); + P3DInstance *inst = inst_mgr->validate_instance(instance); + P3D_object *result = NULL; + if (inst != NULL) { + result = inst->get_panda_script_object(); + } RELEASE_LOCK(_api_lock); return result; @@ -343,7 +358,11 @@ P3D_instance_set_browser_script_object(P3D_instance *instance, assert(P3DInstanceManager::get_global_ptr()->is_initialized()); ACQUIRE_LOCK(_api_lock); - ((P3DInstance *)instance)->set_browser_script_object(object); + P3DInstanceManager *inst_mgr = P3DInstanceManager::get_global_ptr(); + P3DInstance *inst = inst_mgr->validate_instance(instance); + if (inst != NULL) { + inst->set_browser_script_object(object); + } RELEASE_LOCK(_api_lock); } @@ -353,7 +372,14 @@ P3D_request * P3D_instance_get_request(P3D_instance *instance) { assert(P3DInstanceManager::get_global_ptr()->is_initialized()); ACQUIRE_LOCK(_api_lock); - P3D_request *result = ((P3DInstance *)instance)->get_request(); + + P3DInstanceManager *inst_mgr = P3DInstanceManager::get_global_ptr(); + P3DInstance *inst = inst_mgr->validate_instance(instance); + P3D_request *result = NULL; + if (inst != NULL) { + result = inst->get_request(); + } + RELEASE_LOCK(_api_lock); return result; } @@ -387,7 +413,11 @@ P3D_request_finish(P3D_request *request, bool handled) { assert(P3DInstanceManager::get_global_ptr()->is_initialized()); ACQUIRE_LOCK(_api_lock); if (request != (P3D_request *)NULL) { - ((P3DInstance *)request->_instance)->finish_request(request, handled); + P3DInstanceManager *inst_mgr = P3DInstanceManager::get_global_ptr(); + P3DInstance *inst = inst_mgr->validate_instance(request->_instance); + if (inst != NULL) { + inst->finish_request(request, handled); + } } RELEASE_LOCK(_api_lock); } @@ -401,10 +431,17 @@ P3D_instance_feed_url_stream(P3D_instance *instance, int unique_id, size_t this_data_size) { assert(P3DInstanceManager::get_global_ptr()->is_initialized()); ACQUIRE_LOCK(_api_lock); - bool result = ((P3DInstance *)instance)-> - feed_url_stream(unique_id, result_code, http_status_code, - total_expected_data, - (const unsigned char *)this_data, this_data_size); + + P3DInstanceManager *inst_mgr = P3DInstanceManager::get_global_ptr(); + P3DInstance *inst = inst_mgr->validate_instance(instance); + bool result = false; + if (inst != NULL) { + result = inst-> + feed_url_stream(unique_id, result_code, http_status_code, + total_expected_data, + (const unsigned char *)this_data, this_data_size); + } + RELEASE_LOCK(_api_lock); return result; } @@ -413,7 +450,14 @@ bool P3D_instance_handle_event(P3D_instance *instance, P3D_event_data event) { assert(P3DInstanceManager::get_global_ptr()->is_initialized()); ACQUIRE_LOCK(_api_lock); - bool result = ((P3DInstance *)instance)->handle_event(event); + + P3DInstanceManager *inst_mgr = P3DInstanceManager::get_global_ptr(); + P3DInstance *inst = inst_mgr->validate_instance(instance); + bool result = false; + if (inst != NULL) { + result = inst->handle_event(event); + } + RELEASE_LOCK(_api_lock); return result; }