src/hotspot/share/classfile/verifier.cpp
71 static void* verify_byte_codes_fn() {
72 if (OrderAccess::load_acquire(&_verify_byte_codes_fn) == NULL) {
73 void *lib_handle = os::native_java_library();
74 void *func = os::dll_lookup(lib_handle, "VerifyClassCodesForMajorVersion");
75 OrderAccess::release_store(&_verify_byte_codes_fn, func);
76 if (func == NULL) {
77 _is_new_verify_byte_codes_fn = false;
78 func = os::dll_lookup(lib_handle, "VerifyClassCodes");
79 OrderAccess::release_store(&_verify_byte_codes_fn, func);
80 }
81 }
82 return (void*)_verify_byte_codes_fn;
83 }
[pre-existing]
I think this code has race problems; a caller could unexpectedly and
inappropriately return NULL. Consider the case where there is no
VerifyClassCodesForMajorVersion, but there is VerifyClassCodes.
The variable is initially NULL.
Both Thread1 and Thread2 reach line 73, having both seen a NULL value
for the variable.
Thread1 reaches line 80, setting the variable to VerifyClassCodes.
Thread2 reaches line 76, resetting the variable to NULL.
Thread1 reads the now (momentarily) NULL value and returns it.
I think the first release_store should be conditional on func != NULL.
Also, the usage of _is_new_verify_byte_codes_fn seems suspect.
And a minor additional nit: the cast in the return is unnecessary.
71 static void* verify_byte_codes_fn() {
72 if (OrderAccess::load_acquire(&_verify_byte_codes_fn) == NULL) {
73 void *lib_handle = os::native_java_library();
74 void *func = os::dll_lookup(lib_handle, "VerifyClassCodesForMajorVersion");
75 OrderAccess::release_store(&_verify_byte_codes_fn, func);
76 if (func == NULL) {
77 _is_new_verify_byte_codes_fn = false;
78 func = os::dll_lookup(lib_handle, "VerifyClassCodes");
79 OrderAccess::release_store(&_verify_byte_codes_fn, func);
80 }
81 }
82 return (void*)_verify_byte_codes_fn;
83 }
[pre-existing]
I think this code has race problems; a caller could unexpectedly and
inappropriately return NULL. Consider the case where there is no
VerifyClassCodesForMajorVersion, but there is VerifyClassCodes.
The variable is initially NULL.
Both Thread1 and Thread2 reach line 73, having both seen a NULL value
for the variable.
Thread1 reaches line 80, setting the variable to VerifyClassCodes.
Thread2 reaches line 76, resetting the variable to NULL.
Thread1 reads the now (momentarily) NULL value and returns it.
I think the first release_store should be conditional on func != NULL.
Also, the usage of _is_new_verify_byte_codes_fn seems suspect.
And a minor additional nit: the cast in the return is unnecessary.