From e0f104163919db1ae6797b730ca109d1816d85ec Mon Sep 17 00:00:00 2001 From: "brettw@chromium.org" Date: Fri, 22 Jun 2012 18:15:28 +0000 Subject: Try to fix a crash in the var tracking. I think what is happening is that the list is mutated as we iterate over it. This is actually quite a resonable thing to do: we're deleting objects, and those objects may hold refs to other objects in our list. BUG=http://crbug.com/133951 Review URL: https://chromiumcodereview.appspot.com/10633019 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@143644 0039d316-1c4b-4281-b951-d872f2087c98 --- ppapi/proxy/plugin_var_tracker.cc | 51 +++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 21 deletions(-) (limited to 'ppapi/proxy/plugin_var_tracker.cc') diff --git a/ppapi/proxy/plugin_var_tracker.cc b/ppapi/proxy/plugin_var_tracker.cc index 52ecad8..384eb3ec 100644 --- a/ppapi/proxy/plugin_var_tracker.cc +++ b/ppapi/proxy/plugin_var_tracker.cc @@ -153,30 +153,39 @@ void PluginVarTracker::ReleaseHostObject(PluginDispatcher* dispatcher, } void PluginVarTracker::DidDeleteInstance(PP_Instance instance) { + // Calling the destructors on plugin objects may in turn release other + // objects which will mutate the map out from under us. So do a two-step + // process of identifying the ones to delete, and then delete them. + // // See the comment above user_data_to_plugin_ in the header file. We assume // there aren't that many objects so a brute-force search is reasonable. - UserDataToPluginImplementedVarMap::iterator i = - user_data_to_plugin_.begin(); - while (i != user_data_to_plugin_.end()) { - if (i->second.instance == instance) { - if (!i->second.plugin_object_id) { - // This object is for the freed instance and the plugin is not holding - // any references to it. Deallocate immediately. - UserDataToPluginImplementedVarMap::iterator to_remove = i; - ++i; - to_remove->second.ppp_class->Deallocate(to_remove->first); - user_data_to_plugin_.erase(to_remove); - } else { - // The plugin is holding refs to this object. We don't want to call - // Deallocate since the plugin may be depending on those refs to keep - // its data alive. To avoid crashes in this case, just clear out the - // instance to mark it and continue. When the plugin refs go to 0, - // we'll notice there is no instance and call Deallocate. - i->second.instance = 0; - ++i; - } + std::vector user_data_to_delete; + for (UserDataToPluginImplementedVarMap::const_iterator i = + user_data_to_plugin_.begin(); + i != user_data_to_plugin_.end(); + ++i) { + if (i->second.instance == instance) + user_data_to_delete.push_back(i->first); + } + + for (size_t i = 0; i < user_data_to_delete.size(); i++) { + UserDataToPluginImplementedVarMap::iterator found = + user_data_to_plugin_.find(user_data_to_delete[i]); + if (found == user_data_to_plugin_.end()) + continue; // Object removed from list while we were iterating. + + if (!found->second.plugin_object_id) { + // This object is for the freed instance and the plugin is not holding + // any references to it. Deallocate immediately. + found->second.ppp_class->Deallocate(found->first); + user_data_to_plugin_.erase(found); } else { - ++i; + // The plugin is holding refs to this object. We don't want to call + // Deallocate since the plugin may be depending on those refs to keep + // its data alive. To avoid crashes in this case, just clear out the + // instance to mark it and continue. When the plugin refs go to 0, + // we'll notice there is no instance and call Deallocate. + found->second.instance = 0; } } } -- cgit v1.1