-
-
Notifications
You must be signed in to change notification settings - Fork 23.5k
Fix crash in Jolt when doing incremental builds #111408
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
Conversation
I had the same issue this morning as well, clean build fixed it (editor build on macOS, for the reference). |
|
Thanks! |
We experienced first hand why it's called unsafe, and so we should leave it as an explicit choice for contributors, informing themselves of the caveats. See godotengine#111408.
|
Note that instead of doing a |
#111411 will remove the implicit However, I guess it's theoretically possible for someone to run into trouble if they bisect with In any case, they have at least made the explicit choice of entering the "danger zone" by enabling |
We experienced first hand why it's called unsafe, and so we should leave it as an explicit choice for contributors, informing themselves of the caveats. See godotengine#111408. (cherry picked from commit fa57282)
We experienced first hand why it's called unsafe, and so we should leave it as an explicit choice for contributors, informing themselves of the caveats. See godotengine#111408.
We experienced first hand why it's called unsafe, and so we should leave it as an explicit choice for contributors, informing themselves of the caveats. See godotengine#111408. (cherry picked from commit fa57282)
We experienced first hand why it's called unsafe, and so we should leave it as an explicit choice for contributors, informing themselves of the caveats. See godotengine#111408.
We experienced first hand why it's called unsafe, and so we should leave it as an explicit choice for contributors, informing themselves of the caveats. See godotengine#111408. (cherry picked from commit fa57282)
We've had at least one report of someone crashing here after doing an incremental build after #110965 was merged:
godot/thirdparty/jolt_physics/Jolt/RegisterTypes.cpp
Lines 72 to 90 in d61cd91
The reason for this isn't confirmed, but is (as discussed in #110965) potentially as follows:
thirdparty/jolt_physics(and possibly other third-party vendorings as well).CPPPATH, it likely clashes with us relying onfast_unsafe=yesby default whendev_build=yes, which enables theimplicit_cacheSCons option, which in its documentation states that it causes SCons to "ignore any changes that may have been made to search paths (like $CPPPATH or $LIBPATH)".Jolt/RegisterTypes.cppdidn't have any changes to it, it should still have been recompiled, since its header dependencies did change, but these dependencies were likely not restored because offast_unsafe=yes.std::abortin its version check because the compilation environment of that particular file doesn't line up with the caller's compilation environment, leading to this crash.While the fix here is as simple as doing a clean rebuild, this will likely keep happening to people in the future as they go to bisect with
dev_build=yes, since incremental builds from any commit between 1f56d96 and 9052d31 will be subject to this.As such, this PR cherrypicks jrouwe/JoltPhysics@9e48d59 in order to have some kind of change to
Jolt/RegisterTypes.cpp, which should at least prevent the crash from happening going forward.Note that the crash is arguably a red herring here, the real problem is ending up with incomplete incremental builds because of what is likely to be
fast_unsafe=yes, potentially across several third-party vendorings.