Skip to content

Commit

Permalink
Add renaming of PDB files to avoid blocking them
Browse files Browse the repository at this point in the history
  • Loading branch information
DmitriySalnikov committed Apr 4, 2024
1 parent f6a78f8 commit b73e740
Show file tree
Hide file tree
Showing 18 changed files with 411 additions and 77 deletions.
61 changes: 6 additions & 55 deletions core/extension/gdextension.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -729,53 +729,18 @@ GDExtensionInterfaceFunctionPtr GDExtension::get_interface_function(const String

Error GDExtension::open_library(const String &p_path, const String &p_entry_symbol) {
String abs_path = ProjectSettings::get_singleton()->globalize_path(p_path);
#if defined(WINDOWS_ENABLED) && defined(TOOLS_ENABLED)
// If running on the editor on Windows, we copy the library and open the copy.
// This is so the original file isn't locked and can be updated by a compiler.
bool library_copied = false;
if (Engine::get_singleton()->is_editor_hint()) {
if (!FileAccess::exists(abs_path)) {
ERR_PRINT("GDExtension library not found: " + abs_path);
return ERR_FILE_NOT_FOUND;
}

// Copy the file to the same directory as the original with a prefix in the name.
// This is so relative path to dependencies are satisfied.
String copy_path = abs_path.get_base_dir().path_join("~" + abs_path.get_file());
String actual_lib_path;
Error err = OS::get_singleton()->open_dynamic_library(abs_path, library, true, &actual_lib_path, Engine::get_singleton()->is_editor_hint());

// If there's a left-over copy (possibly from a crash) then delete it first.
if (FileAccess::exists(copy_path)) {
DirAccess::remove_absolute(copy_path);
}

Error copy_err = DirAccess::copy_absolute(abs_path, copy_path);
if (copy_err) {
ERR_PRINT("Error copying GDExtension library: " + abs_path);
return ERR_CANT_CREATE;
}
FileAccess::set_hidden_attribute(copy_path, true);
library_copied = true;

// Save the copied path so it can be deleted later.
temp_lib_path = copy_path;

// Use the copy to open the library.
abs_path = copy_path;
if (actual_lib_path.get_file() != abs_path.get_file()) {
// If temporary files are generated, let's change the library path to point at the original,
// because that's what we want to check to see if it's changed.
library_path = actual_lib_path.get_base_dir().path_join(p_path.get_file());
}
#endif

Error err = OS::get_singleton()->open_dynamic_library(abs_path, library, true, &library_path);
ERR_FAIL_COND_V_MSG(err == ERR_FILE_NOT_FOUND, err, "GDExtension dynamic library not found: " + abs_path);
ERR_FAIL_COND_V_MSG(err != OK, err, "Can't open GDExtension dynamic library: " + abs_path);

#if defined(WINDOWS_ENABLED) && defined(TOOLS_ENABLED)
// If we copied the file, let's change the library path to point at the original,
// because that's what we want to check to see if it's changed.
if (library_copied) {
library_path = library_path.get_base_dir() + "\\" + p_path.get_file();
}
#endif

void *entry_funcptr = nullptr;

err = OS::get_singleton()->get_dynamic_library_symbol_handle(library, p_entry_symbol, entry_funcptr, false);
Expand Down Expand Up @@ -803,13 +768,6 @@ void GDExtension::close_library() {
ERR_FAIL_NULL(library);
OS::get_singleton()->close_dynamic_library(library);

#if defined(TOOLS_ENABLED) && defined(WINDOWS_ENABLED)
// Delete temporary copy of library if it exists.
if (!temp_lib_path.is_empty() && Engine::get_singleton()->is_editor_hint()) {
DirAccess::remove_absolute(temp_lib_path);
}
#endif

library = nullptr;
class_icon_paths.clear();

Expand Down Expand Up @@ -1014,13 +972,6 @@ Error GDExtensionResourceLoader::load_gdextension_resource(const String &p_path,

err = p_extension->open_library(is_static_library ? String() : library_path, entry_symbol);
if (err != OK) {
#if defined(WINDOWS_ENABLED) && defined(TOOLS_ENABLED)
// If the DLL fails to load, make sure that temporary DLL copies are cleaned up.
if (Engine::get_singleton()->is_editor_hint()) {
DirAccess::remove_absolute(p_extension->get_temp_library_path());
}
#endif

// Unreference the extension so that this loading can be considered a failure.
p_extension.unref();

Expand Down
7 changes: 0 additions & 7 deletions core/extension/gdextension.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,6 @@ class GDExtension : public Resource {

void *library = nullptr; // pointer if valid,
String library_path;
#if defined(WINDOWS_ENABLED) && defined(TOOLS_ENABLED)
String temp_lib_path;
#endif
bool reloadable = false;

struct Extension {
Expand Down Expand Up @@ -130,10 +127,6 @@ class GDExtension : public Resource {
Error open_library(const String &p_path, const String &p_entry_symbol);
void close_library();

#if defined(WINDOWS_ENABLED) && defined(TOOLS_ENABLED)
String get_temp_library_path() const { return temp_lib_path; }
#endif

enum InitializationLevel {
INITIALIZATION_LEVEL_CORE = GDEXTENSION_INITIALIZATION_CORE,
INITIALIZATION_LEVEL_SERVERS = GDEXTENSION_INITIALIZATION_SERVERS,
Expand Down
2 changes: 1 addition & 1 deletion core/os/os.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ class OS {

virtual void alert(const String &p_alert, const String &p_title = "ALERT!");

virtual Error open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path = false, String *r_resolved_path = nullptr) { return ERR_UNAVAILABLE; }
virtual Error open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path = false, String *r_resolved_path = nullptr, bool p_generate_temp_files = false) { return ERR_UNAVAILABLE; }
virtual Error close_dynamic_library(void *p_library_handle) { return ERR_UNAVAILABLE; }
virtual Error get_dynamic_library_symbol_handle(void *p_library_handle, const String &p_name, void *&p_symbol_handle, bool p_optional = false) { return ERR_UNAVAILABLE; }

Expand Down
2 changes: 1 addition & 1 deletion drivers/unix/os_unix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,7 @@ String OS_Unix::get_locale() const {
return locale;
}

Error OS_Unix::open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path, String *r_resolved_path) {
Error OS_Unix::open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path, String *r_resolved_path, bool p_generate_temp_files) {
String path = p_path;

if (FileAccess::exists(path) && path.is_relative_path()) {
Expand Down
2 changes: 1 addition & 1 deletion drivers/unix/os_unix.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class OS_Unix : public OS {

virtual Error get_entropy(uint8_t *r_buffer, int p_bytes) override;

virtual Error open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path = false, String *r_resolved_path = nullptr) override;
virtual Error open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path = false, String *r_resolved_path = nullptr, bool p_generate_temp_files = false) override;
virtual Error close_dynamic_library(void *p_library_handle) override;
virtual Error get_dynamic_library_symbol_handle(void *p_library_handle, const String &p_name, void *&p_symbol_handle, bool p_optional = false) override;

Expand Down
2 changes: 1 addition & 1 deletion platform/android/os_android.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ Vector<String> OS_Android::get_granted_permissions() const {
return godot_java->get_granted_permissions();
}

Error OS_Android::open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path, String *r_resolved_path) {
Error OS_Android::open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path, String *r_resolved_path, bool p_generate_temp_files) {
String path = p_path;
bool so_file_exists = true;
if (!FileAccess::exists(path)) {
Expand Down
2 changes: 1 addition & 1 deletion platform/android/os_android.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ class OS_Android : public OS_Unix {

virtual void alert(const String &p_alert, const String &p_title) override;

virtual Error open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path = false, String *r_resolved_path = nullptr) override;
virtual Error open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path = false, String *r_resolved_path = nullptr, bool p_generate_temp_files = false) override;

virtual String get_name() const override;
virtual String get_distribution_name() const override;
Expand Down
2 changes: 1 addition & 1 deletion platform/ios/os_ios.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class OS_IOS : public OS_Unix {
virtual Vector<String> get_system_font_path_for_text(const String &p_font_name, const String &p_text, const String &p_locale = String(), const String &p_script = String(), int p_weight = 400, int p_stretch = 100, bool p_italic = false) const override;
virtual String get_system_font_path(const String &p_font_name, int p_weight = 400, int p_stretch = 100, bool p_italic = false) const override;

virtual Error open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path = false, String *r_resolved_path = nullptr) override;
virtual Error open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path = false, String *r_resolved_path = nullptr, bool p_generate_temp_files = false) override;
virtual Error close_dynamic_library(void *p_library_handle) override;
virtual Error get_dynamic_library_symbol_handle(void *p_library_handle, const String &p_name, void *&p_symbol_handle, bool p_optional = false) override;

Expand Down
2 changes: 1 addition & 1 deletion platform/ios/os_ios.mm
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ void register_dynamic_symbol(char *name, void *address) {
return p_path;
}

Error OS_IOS::open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path, String *r_resolved_path) {
Error OS_IOS::open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path, String *r_resolved_path, bool p_generate_temp_files) {
if (p_path.length() == 0) {
// Static xcframework.
p_library_handle = RTLD_SELF;
Expand Down
2 changes: 1 addition & 1 deletion platform/macos/os_macos.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class OS_MacOS : public OS_Unix {

virtual void alert(const String &p_alert, const String &p_title = "ALERT!") override;

virtual Error open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path = false, String *r_resolved_path = nullptr) override;
virtual Error open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path = false, String *r_resolved_path = nullptr, bool p_generate_temp_files = false) override;

virtual MainLoop *get_main_loop() const override;

Expand Down
2 changes: 1 addition & 1 deletion platform/macos/os_macos.mm
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@
return p_path;
}

Error OS_MacOS::open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path, String *r_resolved_path) {
Error OS_MacOS::open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path, String *r_resolved_path, bool p_generate_temp_files) {
String path = get_framework_executable(p_path);

if (!FileAccess::exists(path)) {
Expand Down
2 changes: 1 addition & 1 deletion platform/web/os_web.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ bool OS_Web::is_userfs_persistent() const {
return idb_available;
}

Error OS_Web::open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path, String *r_resolved_path) {
Error OS_Web::open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path, String *r_resolved_path, bool p_generate_temp_files) {
String path = p_path.get_file();
p_library_handle = dlopen(path.utf8().get_data(), RTLD_NOW);
ERR_FAIL_NULL_V_MSG(p_library_handle, ERR_CANT_OPEN, vformat("Can't open dynamic library: %s. Error: %s.", p_path, dlerror()));
Expand Down
2 changes: 1 addition & 1 deletion platform/web/os_web.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ class OS_Web : public OS_Unix {

void alert(const String &p_alert, const String &p_title = "ALERT!") override;

Error open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path = false, String *r_resolved_path = nullptr) override;
Error open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path = false, String *r_resolved_path = nullptr, bool p_generate_temp_files = false) override;

void resume_audio();

Expand Down
1 change: 1 addition & 0 deletions platform/windows/SCsub
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ common_win = [
"joypad_windows.cpp",
"tts_windows.cpp",
"windows_terminal_logger.cpp",
"windows_utils.cpp",
"native_menu_windows.cpp",
"gl_manager_windows_native.cpp",
"gl_manager_windows_angle.cpp",
Expand Down
63 changes: 60 additions & 3 deletions platform/windows/os_windows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "joypad_windows.h"
#include "lang_table.h"
#include "windows_terminal_logger.h"
#include "windows_utils.h"

#include "core/debugger/engine_debugger.h"
#include "core/debugger/script_debugger.h"
Expand Down Expand Up @@ -269,6 +270,10 @@ void OS_Windows::finalize() {
}

void OS_Windows::finalize_core() {
while (!temp_libraries.is_empty()) {
_remove_temp_library(temp_libraries.last()->key);
}

FileAccessWindows::finalize();

timeEndPeriod(1);
Expand Down Expand Up @@ -354,7 +359,7 @@ void debug_dynamic_library_check_dependencies(const String &p_root_path, const S
}
#endif

Error OS_Windows::open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path, String *r_resolved_path) {
Error OS_Windows::open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path, String *r_resolved_path, bool p_generate_temp_files) {
String path = p_path.replace("/", "\\");

if (!FileAccess::exists(path)) {
Expand All @@ -364,6 +369,35 @@ Error OS_Windows::open_dynamic_library(const String &p_path, void *&p_library_ha

ERR_FAIL_COND_V(!FileAccess::exists(path), ERR_FILE_NOT_FOUND);

// Here we want a copy to be loaded.
// This is so the original file isn't locked and can be updated by a compiler.
if (p_generate_temp_files) {
// Copy the file to the same directory as the original with a prefix in the name.
// This is so relative path to dependencies are satisfied.
String copy_path = path.get_base_dir().path_join("~" + path.get_file());

// If there's a left-over copy (possibly from a crash) then delete it first.
if (FileAccess::exists(copy_path)) {
DirAccess::remove_absolute(copy_path);
}

Error copy_err = DirAccess::copy_absolute(path, copy_path);
if (copy_err) {
ERR_PRINT("Error copying library: " + path);
return ERR_CANT_CREATE;
}

FileAccess::set_hidden_attribute(copy_path, true);

// Save the copied path so it can be deleted later.
path = copy_path;

Error pdb_err = WindowsUtils::copy_and_rename_pdb(path);
if (pdb_err != OK && pdb_err != ERR_SKIP) {
WARN_PRINT(vformat("Failed to rename the PDB file. The original PDB file for '%s' will be loaded.", path));
}
}

typedef DLL_DIRECTORY_COOKIE(WINAPI * PAddDllDirectory)(PCWSTR);
typedef BOOL(WINAPI * PRemoveDllDirectory)(DLL_DIRECTORY_COOKIE);

Expand All @@ -378,8 +412,12 @@ Error OS_Windows::open_dynamic_library(const String &p_path, void *&p_library_ha
}

p_library_handle = (void *)LoadLibraryExW((LPCWSTR)(path.utf16().get_data()), nullptr, (p_also_set_library_path && has_dll_directory_api) ? LOAD_LIBRARY_SEARCH_DEFAULT_DIRS : 0);
#ifdef DEBUG_ENABLED
if (!p_library_handle) {
if (p_generate_temp_files) {
DirAccess::remove_absolute(path);
}

#ifdef DEBUG_ENABLED
DWORD err_code = GetLastError();

HashSet<String> checekd_libs;
Expand All @@ -397,8 +435,10 @@ Error OS_Windows::open_dynamic_library(const String &p_path, void *&p_library_ha
} else {
ERR_FAIL_V_MSG(ERR_CANT_OPEN, vformat("Can't open dynamic library: %s. Error: %s.", p_path, format_error_message(err_code)));
}
#endif
}
#else

#ifndef DEBUG_ENABLED
ERR_FAIL_NULL_V_MSG(p_library_handle, ERR_CANT_OPEN, vformat("Can't open dynamic library: %s. Error: %s.", p_path, format_error_message(GetLastError())));
#endif

Expand All @@ -410,16 +450,33 @@ Error OS_Windows::open_dynamic_library(const String &p_path, void *&p_library_ha
*r_resolved_path = path;
}

if (p_generate_temp_files) {
temp_libraries[p_library_handle] = path;
}

return OK;
}

Error OS_Windows::close_dynamic_library(void *p_library_handle) {
if (!FreeLibrary((HMODULE)p_library_handle)) {
return FAILED;
}

// Delete temporary copy of library if it exists.
_remove_temp_library(p_library_handle);

return OK;
}

void OS_Windows::_remove_temp_library(void *p_library_handle) {
if (temp_libraries.has(p_library_handle)) {
String path = temp_libraries[p_library_handle];
DirAccess::remove_absolute(path);
WindowsUtils::remove_temp_pdbs(path);
temp_libraries.erase(p_library_handle);
}
}

Error OS_Windows::get_dynamic_library_symbol_handle(void *p_library_handle, const String &p_name, void *&p_symbol_handle, bool p_optional) {
p_symbol_handle = (void *)GetProcAddress((HMODULE)p_library_handle, p_name.utf8().get_data());
if (!p_symbol_handle) {
Expand Down
5 changes: 4 additions & 1 deletion platform/windows/os_windows.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ class OS_Windows : public OS {
bool dwrite_init = false;
bool dwrite2_init = false;

HashMap<void *, String> temp_libraries;

void _remove_temp_library(void *p_library_handle);
String _get_default_fontname(const String &p_font_name) const;
DWRITE_FONT_WEIGHT _weight_to_dw(int p_weight) const;
DWRITE_FONT_STRETCH _stretch_to_dw(int p_stretch) const;
Expand Down Expand Up @@ -155,7 +158,7 @@ class OS_Windows : public OS {

virtual Error get_entropy(uint8_t *r_buffer, int p_bytes) override;

virtual Error open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path = false, String *r_resolved_path = nullptr) override;
virtual Error open_dynamic_library(const String &p_path, void *&p_library_handle, bool p_also_set_library_path = false, String *r_resolved_path = nullptr, bool p_generate_temp_files = false) override;
virtual Error close_dynamic_library(void *p_library_handle) override;
virtual Error get_dynamic_library_symbol_handle(void *p_library_handle, const String &p_name, void *&p_symbol_handle, bool p_optional = false) override;

Expand Down
Loading

0 comments on commit b73e740

Please sign in to comment.