On Tue, 23 Mar 2021 05:52:51 GMT, Ioi Lam <ik...@openjdk.org> wrote: >> src/hotspot/share/prims/jvmtiRedefineClasses.hpp line 484: >> >>> 482: void rewrite_cp_refs_in_method(methodHandle method, >>> 483: methodHandle * new_method_p, TRAPS); >>> 484: bool rewrite_cp_refs_in_methods(InstanceKlass* scratch_class, TRAPS); >> >> This method clears any pending exception and so should not be a TRAPS method. > > `VM_RedefineClasses::load_new_class_versions` also seems to never throw. > These functions should be changed to take a `Thread*` parameter, and should > use `HandleMark em(thread);` to guarantee that an exception never leaves the > function.
Both of these functions are good examples of the convention that we're trying to agree on. In, load_new_class_versions, TRAPS is passed to materialize THREAD. THREAD is used then in a lot of places, and also to pass to SystemDictionary::parse_stream(...TRAPS), which does have an exceptional return that must be handled. Removing TRAPS then adding: JavaThread* current = JavaThread::current(); changing THREAD to current in most of the places seems ok, but passing 'current' to SystemDictionary::resolve_from_stream loses the information visually that this function returns an exception that must be handled. We need some meta-writeup rather than these decisions made in pull requests, because if we made these decisions collectively, I missed out. ------------- PR: https://git.openjdk.java.net/jdk/pull/3141
