diff options
author | brettw@chromium.org <brettw@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-06-22 18:15:28 +0000 |
---|---|---|
committer | brettw@chromium.org <brettw@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-06-22 18:15:28 +0000 |
commit | e0f104163919db1ae6797b730ca109d1816d85ec (patch) | |
tree | 9b92b22e11f2d9a8c9f86f6785a473797b5b0421 /ppapi/proxy | |
parent | b1af0267398a93b62497de11d2bb833476e6997c (diff) | |
download | chromium_src-e0f104163919db1ae6797b730ca109d1816d85ec.zip chromium_src-e0f104163919db1ae6797b730ca109d1816d85ec.tar.gz chromium_src-e0f104163919db1ae6797b730ca109d1816d85ec.tar.bz2 |
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
Diffstat (limited to 'ppapi/proxy')
-rw-r--r-- | ppapi/proxy/plugin_var_tracker.cc | 51 |
1 files changed, 30 insertions, 21 deletions
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<void*> 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; } } } |