Skip to content

Commit

Permalink
8315220: Event NativeLibraryLoad breaks invariant by taking a stacktr…
Browse files Browse the repository at this point in the history
…ace when thread is in state _thread_in_native

Reviewed-by: mbaesken, egahlin
  • Loading branch information
Markus Grönlund committed Sep 7, 2023
1 parent 8f7e29b commit 1cae0f5
Show file tree
Hide file tree
Showing 10 changed files with 281 additions and 152 deletions.
25 changes: 5 additions & 20 deletions src/hotspot/os/aix/os_aix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
#include "utilities/growableArray.hpp"
#include "utilities/vmError.hpp"
#if INCLUDE_JFR
#include "jfr/jfrEvents.hpp"
#include "jfr/support/jfrNativeLibraryLoadEvent.hpp"
#endif

// put OS-includes here (sorted alphabetically)
Expand Down Expand Up @@ -1116,11 +1116,6 @@ void *os::dll_load(const char *filename, char *ebuf, int ebuflen) {
return nullptr;
}

#if INCLUDE_JFR
EventNativeLibraryLoad event;
event.set_name(filename);
#endif

// RTLD_LAZY has currently the same behavior as RTLD_NOW
// The dl is loaded immediately with all its dependants.
int dflags = RTLD_LAZY;
Expand All @@ -1131,19 +1126,14 @@ void *os::dll_load(const char *filename, char *ebuf, int ebuflen) {
dflags |= RTLD_MEMBER;
}

void * result= ::dlopen(filename, dflags);
void* result;
JFR_ONLY(NativeLibraryLoadEvent load_event(filename, &result);)
result = ::dlopen(filename, dflags);
if (result != nullptr) {
Events::log_dll_message(nullptr, "Loaded shared library %s", filename);
// Reload dll cache. Don't do this in signal handling.
LoadedLibraries::reload();
log_info(os)("shared library load of %s was successful", filename);

#if INCLUDE_JFR
event.set_success(true);
event.set_errorMessage(nullptr);
event.commit();
#endif

return result;
} else {
// error analysis when dlopen fails
Expand All @@ -1157,12 +1147,7 @@ void *os::dll_load(const char *filename, char *ebuf, int ebuflen) {
}
Events::log_dll_message(nullptr, "Loading shared library %s failed, %s", filename, error_report);
log_info(os)("shared library load of %s failed, %s", filename, error_report);

#if INCLUDE_JFR
event.set_success(false);
event.set_errorMessage(error_report);
event.commit();
#endif
JFR_ONLY(load_event.set_error_msg(error_report);)
}
return nullptr;
}
Expand Down
41 changes: 9 additions & 32 deletions src/hotspot/os/bsd/os_bsd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
#include "utilities/vmError.hpp"
#if INCLUDE_JFR
#include "jfr/jfrEvents.hpp"
#include "jfr/support/jfrNativeLibraryLoadEvent.hpp"
#endif

// put OS-includes here
Expand Down Expand Up @@ -979,21 +980,13 @@ void * os::dll_load(const char *filename, char *ebuf, int ebuflen) {
#else
log_info(os)("attempting shared library load of %s", filename);

#if INCLUDE_JFR
EventNativeLibraryLoad event;
event.set_name(filename);
#endif

void * result= ::dlopen(filename, RTLD_LAZY);
void* result;
JFR_ONLY(NativeLibraryLoadEvent load_event(filename, &result);)
result = ::dlopen(filename, RTLD_LAZY);
if (result != nullptr) {
Events::log_dll_message(nullptr, "Loaded shared library %s", filename);
// Successful loading
log_info(os)("shared library load of %s was successful", filename);
#if INCLUDE_JFR
event.set_success(true);
event.set_errorMessage(nullptr);
event.commit();
#endif
return result;
}

Expand All @@ -1008,11 +1001,7 @@ void * os::dll_load(const char *filename, char *ebuf, int ebuflen) {
}
Events::log_dll_message(nullptr, "Loading shared library %s failed, %s", filename, error_report);
log_info(os)("shared library load of %s failed, %s", filename, error_report);
#if INCLUDE_JFR
event.set_success(false);
event.set_errorMessage(error_report);
event.commit();
#endif
JFR_ONLY(load_event.set_error_msg(error_report);)

return nullptr;
#endif // STATIC_BUILD
Expand All @@ -1024,21 +1013,13 @@ void * os::dll_load(const char *filename, char *ebuf, int ebuflen) {
#else
log_info(os)("attempting shared library load of %s", filename);

#if INCLUDE_JFR
EventNativeLibraryLoad event;
event.set_name(filename);
#endif

void * result= ::dlopen(filename, RTLD_LAZY);
void* result;
JFR_ONLY(NativeLibraryLoadEvent load_event(filename, &result);)
result = ::dlopen(filename, RTLD_LAZY);
if (result != nullptr) {
Events::log_dll_message(nullptr, "Loaded shared library %s", filename);
// Successful loading
log_info(os)("shared library load of %s was successful", filename);
#if INCLUDE_JFR
event.set_success(true);
event.set_errorMessage(nullptr);
event.commit();
#endif
return result;
}

Expand All @@ -1055,11 +1036,7 @@ void * os::dll_load(const char *filename, char *ebuf, int ebuflen) {
}
Events::log_dll_message(nullptr, "Loading shared library %s failed, %s", filename, error_report);
log_info(os)("shared library load of %s failed, %s", filename, error_report);
#if INCLUDE_JFR
event.set_success(false);
event.set_errorMessage(error_report);
event.commit();
#endif
JFR_ONLY(load_event.set_error_msg(error_report);)
int diag_msg_max_length=ebuflen-strlen(ebuf);
char* diag_msg_buf=ebuf+strlen(ebuf);

Expand Down
25 changes: 6 additions & 19 deletions src/hotspot/os/linux/os_linux.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
#include "utilities/vmError.hpp"
#if INCLUDE_JFR
#include "jfr/jfrEvents.hpp"
#include "jfr/support/jfrNativeLibraryLoadEvent.hpp"
#endif

// put OS-includes here
Expand Down Expand Up @@ -1794,15 +1795,10 @@ void * os::dll_load(const char *filename, char *ebuf, int ebuflen) {
return nullptr;
}

void * os::Linux::dlopen_helper(const char *filename, char *ebuf,
int ebuflen) {
void * result = ::dlopen(filename, RTLD_LAZY);

#if INCLUDE_JFR
EventNativeLibraryLoad event;
event.set_name(filename);
#endif

void * os::Linux::dlopen_helper(const char *filename, char *ebuf, int ebuflen) {
void* result;
JFR_ONLY(NativeLibraryLoadEvent load_event(filename, &result);)
result = ::dlopen(filename, RTLD_LAZY);
if (result == nullptr) {
const char* error_report = ::dlerror();
if (error_report == nullptr) {
Expand All @@ -1814,19 +1810,10 @@ void * os::Linux::dlopen_helper(const char *filename, char *ebuf,
}
Events::log_dll_message(nullptr, "Loading shared library %s failed, %s", filename, error_report);
log_info(os)("shared library load of %s failed, %s", filename, error_report);
#if INCLUDE_JFR
event.set_success(false);
event.set_errorMessage(error_report);
event.commit();
#endif
JFR_ONLY(load_event.set_error_msg(error_report);)
} else {
Events::log_dll_message(nullptr, "Loaded shared library %s", filename);
log_info(os)("shared library load of %s was successful", filename);
#if INCLUDE_JFR
event.set_success(true);
event.set_errorMessage(nullptr);
event.commit();
#endif
}
return result;
}
Expand Down
19 changes: 4 additions & 15 deletions src/hotspot/os/posix/os_posix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
#include "utilities/macros.hpp"
#include "utilities/vmError.hpp"
#if INCLUDE_JFR
#include "jfr/jfrEvents.hpp"
#include "jfr/support/jfrNativeLibraryLoadEvent.hpp"
#endif

#ifdef AIX
Expand Down Expand Up @@ -728,10 +728,7 @@ void os::dll_unload(void *lib) {
}
#endif // LINUX

#if INCLUDE_JFR
EventNativeLibraryUnload event;
event.set_name(l_path);
#endif
JFR_ONLY(NativeLibraryUnloadEvent unload_event(l_path);)

if (l_path == nullptr) {
l_path = "<not available>";
Expand All @@ -742,11 +739,7 @@ void os::dll_unload(void *lib) {
Events::log_dll_message(nullptr, "Unloaded shared library \"%s\" [" INTPTR_FORMAT "]",
l_path, p2i(lib));
log_info(os)("Unloaded shared library \"%s\" [" INTPTR_FORMAT "]", l_path, p2i(lib));
#if INCLUDE_JFR
event.set_success(true);
event.set_errorMessage(nullptr);
event.commit();
#endif
JFR_ONLY(unload_event.set_result(true);)
} else {
const char* error_report = ::dlerror();
if (error_report == nullptr) {
Expand All @@ -757,11 +750,7 @@ void os::dll_unload(void *lib) {
l_path, p2i(lib), error_report);
log_info(os)("Attempt to unload shared library \"%s\" [" INTPTR_FORMAT "] failed, %s",
l_path, p2i(lib), error_report);
#if INCLUDE_JFR
event.set_success(false);
event.set_errorMessage(error_report);
event.commit();
#endif
JFR_ONLY(unload_event.set_error_msg(error_report);)
}
// Update the dll cache
AIX_ONLY(LoadedLibraries::reload());
Expand Down
62 changes: 12 additions & 50 deletions src/hotspot/os/windows/os_windows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
#include "windbghelp.hpp"
#if INCLUDE_JFR
#include "jfr/jfrEvents.hpp"
#include "jfr/support/jfrNativeLibraryLoadEvent.hpp"
#endif

#ifdef _DEBUG
Expand Down Expand Up @@ -1246,33 +1247,22 @@ void os::dll_unload(void *lib) {
snprintf(name, MAX_PATH, "<not available>");
}

#if INCLUDE_JFR
EventNativeLibraryUnload event;
event.set_name(name);
#endif
JFR_ONLY(NativeLibraryUnloadEvent unload_event(name);)

if (::FreeLibrary((HMODULE)lib)) {
Events::log_dll_message(nullptr, "Unloaded dll \"%s\" [" INTPTR_FORMAT "]", name, p2i(lib));
log_info(os)("Unloaded dll \"%s\" [" INTPTR_FORMAT "]", name, p2i(lib));
#if INCLUDE_JFR
event.set_success(true);
event.set_errorMessage(nullptr);
event.commit();
#endif
JFR_ONLY(unload_event.set_result(true);)
} else {
const DWORD errcode = ::GetLastError();
char buf[500];
size_t tl = os::lasterror(buf, sizeof(buf));
Events::log_dll_message(nullptr, "Attempt to unload dll \"%s\" [" INTPTR_FORMAT "] failed (error code %d)", name, p2i(lib), errcode);
log_info(os)("Attempt to unload dll \"%s\" [" INTPTR_FORMAT "] failed (error code %d)", name, p2i(lib), errcode);
#if INCLUDE_JFR
event.set_success(false);
if (tl == 0) {
os::snprintf(buf, sizeof(buf), "Attempt to unload dll failed (error code %d)", (int) errcode);
}
event.set_errorMessage(buf);
event.commit();
#endif
JFR_ONLY(unload_event.set_error_msg(buf);)
}
}

Expand Down Expand Up @@ -1541,21 +1531,14 @@ static int _print_module(const char* fname, address base_address,
// same architecture as Hotspot is running on
void * os::dll_load(const char *name, char *ebuf, int ebuflen) {
log_info(os)("attempting shared library load of %s", name);
#if INCLUDE_JFR
EventNativeLibraryLoad event;
event.set_name(name);
#endif
void * result = LoadLibrary(name);
void* result;
JFR_ONLY(NativeLibraryLoadEvent load_event(name, &result);)
result = LoadLibrary(name);
if (result != nullptr) {
Events::log_dll_message(nullptr, "Loaded shared library %s", name);
// Recalculate pdb search path if a DLL was loaded successfully.
SymbolEngine::recalc_search_path();
log_info(os)("shared library load of %s was successful", name);
#if INCLUDE_JFR
event.set_success(true);
event.set_errorMessage(nullptr);
event.commit();
#endif
return result;
}
DWORD errcode = GetLastError();
Expand All @@ -1569,11 +1552,7 @@ void * os::dll_load(const char *name, char *ebuf, int ebuflen) {
if (errcode == ERROR_MOD_NOT_FOUND) {
strncpy(ebuf, "Can't find dependent libraries", ebuflen - 1);
ebuf[ebuflen - 1] = '\0';
#if INCLUDE_JFR
event.set_success(false);
event.set_errorMessage(ebuf);
event.commit();
#endif
JFR_ONLY(load_event.set_error_msg(ebuf);)
return nullptr;
}

Expand All @@ -1584,11 +1563,7 @@ void * os::dll_load(const char *name, char *ebuf, int ebuflen) {
// else call os::lasterror to obtain system error message
int fd = ::open(name, O_RDONLY | O_BINARY, 0);
if (fd < 0) {
#if INCLUDE_JFR
event.set_success(false);
event.set_errorMessage("open on dll file did not work");
event.commit();
#endif
JFR_ONLY(load_event.set_error_msg("open on dll file did not work");)
return nullptr;
}

Expand All @@ -1615,11 +1590,7 @@ void * os::dll_load(const char *name, char *ebuf, int ebuflen) {
::close(fd);
if (failed_to_get_lib_arch) {
// file i/o error - report os::lasterror(...) msg
#if INCLUDE_JFR
event.set_success(false);
event.set_errorMessage("failed to get lib architecture");
event.commit();
#endif
JFR_ONLY(load_event.set_error_msg("failed to get lib architecture");)
return nullptr;
}

Expand Down Expand Up @@ -1664,11 +1635,7 @@ void * os::dll_load(const char *name, char *ebuf, int ebuflen) {
// If the architecture is right
// but some other error took place - report os::lasterror(...) msg
if (lib_arch == running_arch) {
#if INCLUDE_JFR
event.set_success(false);
event.set_errorMessage("lib architecture matches, but other error occured");
event.commit();
#endif
JFR_ONLY(load_event.set_error_msg("lib architecture matches, but other error occured");)
return nullptr;
}

Expand All @@ -1682,12 +1649,7 @@ void * os::dll_load(const char *name, char *ebuf, int ebuflen) {
"Can't load this .dll (machine code=0x%x) on a %s-bit platform",
lib_arch, running_arch_str);
}
#if INCLUDE_JFR
event.set_success(false);
event.set_errorMessage(ebuf);
event.commit();
#endif

JFR_ONLY(load_event.set_error_msg(ebuf);)
return nullptr;
}

Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/jfr/metadata/metadata.xml
Original file line number Diff line number Diff line change
Expand Up @@ -939,14 +939,14 @@
<Field type="ulong" contentType="address" name="topAddress" label="Top Address" description="Ending address of the module, if available" />
</Event>

<Event name="NativeLibraryLoad" category="Java Virtual Machine, Runtime" label="Native Library Load" thread="false" stackTrace="true" startTime="true"
<Event name="NativeLibraryLoad" category="Java Virtual Machine, Runtime" label="Native Library Load" thread="true" stackTrace="true" startTime="true"
description="Information about a dynamic library or other native image load operation">
<Field type="string" name="name" label="Name" />
<Field type="boolean" name="success" label="Success" description="Success or failure of the load operation" />
<Field type="string" name="errorMessage" label="Error Message" description="In case of a load error, error description" />
</Event>

<Event name="NativeLibraryUnload" category="Java Virtual Machine, Runtime" label="Native Library Unload" thread="false" stackTrace="true" startTime="true"
<Event name="NativeLibraryUnload" category="Java Virtual Machine, Runtime" label="Native Library Unload" thread="true" stackTrace="true" startTime="true"
description="Information about a dynamic library or other native image unload operation">
<Field type="string" name="name" label="Name" />
<Field type="boolean" name="success" label="Success" description="Success or failure of the unload operation" />
Expand Down

1 comment on commit 1cae0f5

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.