RFR: 8258408: SystemDictionary passes TRAPS to functions that don't throw exceptions [v2]
Coleen Phillimore
coleenp at openjdk.java.net
Fri Dec 18 13:13:27 UTC 2020
On Fri, 18 Dec 2020 04:16:31 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> old code vs. new code.
>
> Right so my point is that in the new code:
>
> probe->set_definer(NULL);
> placeholders()->find_and_remove(p_index, p_hash, name_h, loader_data, PlaceholderTable::DEFINE_CLASS, THREAD);
> SystemDictionary_lock->notify_all();
>
> you can now call these while an exception is pending. Whether that is harmless or not depends on the details of that code. I poked around a little and it seems okay but it isn't at all obvious that it is okay.
I spent some time reading through this code as well, because I couldn't see why we'd copy the exception. This is why I changed it, to prevent spending future time reading through this code. The other reason that I don't like declaring TRAPS for functions that don't throw exceptions is because it's unclear quickly at the call site that the placeholders() call can throw an exception, which would overwrite the one that's already in the _pending_exception field. It doesn't. I could have changed the find_and_remove() function to pass THREAD as the first parameter which would make it less suspicious looking, but I had to stop somewhere with this iteration to be nice to code reviewers. Thanks!
-------------
PR: https://git.openjdk.java.net/jdk/pull/1808
More information about the hotspot-dev
mailing list