Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add renaming of PDB files to avoid blocking them #87117

Merged
merged 1 commit into from
Apr 11, 2024
Merged

Add renaming of PDB files to avoid blocking them #87117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 6 additions & 55 deletions core/extension/gdextension.cpp
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading