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.

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

Use-after-free when closing editor while using GDExtension on Windows #95310

Closed
mihe opened this issue Aug 9, 2024 · 1 comment · Fixed by #95311
Closed

Use-after-free when closing editor while using GDExtension on Windows #95310

mihe opened this issue Aug 9, 2024 · 1 comment · Fixed by #95311

Comments

@mihe
Copy link
Contributor

mihe commented Aug 9, 2024

Tested versions

Reproducible in: 4.3.rc [739019e]

System information

Windows 11 (10.0.22631)

Issue description

I haven't bisected this or anything, but given the affected code (WindowsUtils::remove_temp_pdbs) I would assume this is a regression caused by #87117.

Similar to #95306, I only noticed this when running the editor under Application Verifier on Windows, which similar to Valgrind lets you know if a use-after-free has occurred, among other things. I haven't noticed any crash when running normally, but I suppose that could maybe happen, given the somewhat random nature of a use-after-free.

Hopefully the problem is apparent from this flow when closing the editor of a project (on Windows specifically) that utilizes a GDExtension:

  1. Main::cleanup is called
  2. ⚠️ Main::cleanup deletes packed_data (which is the singleton for PackedData) but the singleton isn't zeroed out
  3. Main::cleanup calls unregister_core_types
  4. unregister_core_types deletes gdextension_manager
  5. That unrefs all the GDExtension, thus deleting them
  6. GDExtension::~GDExtension calls GDExtension::close_library
  7. GDExtension::close_library calls OS_Windows::close_dynamic_library
  8. OS_Windows::close_dynamic_library calls OS_Windows::_remove_temp_library
  9. OS_Windows::_remove_temp_library calls WindowsUtils::remove_temp_pdbs
  10. WindowsUtils::remove_temp_pdbs calls FileAccess::exists
  11. ⚠️ FileAccess::exists accesses PackedData::singleton

Steps to reproduce

N/A

Minimal reproduction project (MRP)

I'd be happy to provide an MRP, but I'm fairly sure this should be reproducible in pretty much any godot-cpp project.

@mihe
Copy link
Contributor Author

mihe commented Aug 9, 2024

I put up a fix in #95311.

CC @dsnopek

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants