Principia Crashing in principia__CollisionDeleteExecutor

[jenden0]

In general my game has been pretty unstable, with frequent crashes. This one in particular seemed to have a fairly useful stack trace, so posting it here. I loaded the game and immediately switched to the flight view of an vessel orbiting earth. After changing the plotting frame of reference and time warping a bit I dropped out of timewarp to perform a manual maneuver and the game crashed. No error was displayed.

Not sure if it's related to my previous crash report here. Hopefully this one is a bit more actionable as it contains a full stack trace, though I do acknowledge that the linux distro isn't supported.

INFO.20241121-150731.31589.log
WARNING.20241121-150731.31589.log
ERROR.20241121-150731.31589.log
player_and_ksp_log.zip

Caught fatal signal - signo:6 code:-6 errno:0 addr:0x3e800007b65
Obtained 28 stack frames.
#0  0x007fa82ac55a20 in __sigaction
#1  0x007fa82aca736c in pthread_key_delete
#2  0x007fa82ac55976 in gsignal
#3  0x007fa82ac3d8f7 in abort
#4  0x007fa82c673935 in __gnu_cxx::__verbose_terminate_handler()
#5  0x007fa82c673046 in __cxxabiv1::__terminate(void (*)())
#6  0x007fa82c673091 in std::terminate()
#7  0x007fa82c5eca53 in __cxa_throw
#8  0x007fa47b1a7af2 in std::__1::__throw_bad_optional_access[abi:ue170006]()
#9  0x007fa47b5cce45 in principia__CollisionDeleteExecutor
#10 0x000000418e584b in (wrapper managed-to-native) principia.ksp_plugin_adapter.Interface:CollisionDeleteExecutor (intptr,intptr&,principia.ksp_plugin_adapter.TQP&)
#11 0x0000004149f8f0 in principia.ksp_plugin_adapter.PrincipiaPluginAdapter:RenderTrajectoryCollisions (string,System.Nullable`1<principia.ksp_plugin_adapter.TQP>&,System.Nullable`1<principia.ksp_plugin_adapter.TQP>&)
#12 0x0000004149ebb8 in principia.ksp_plugin_adapter.PrincipiaPluginAdapter:LateUpdate ()
#13 0x00000041483a8a in (wrapper runtime-invoke) object:runtime_invoke_void__this__ (object,intptr,intptr,intptr)
#14 0x007fa82365b695 in mono_print_method_from_ip
#15 0x007fa8237cb128 in mono_perfcounter_foreach
#16 0x007fa8237cbfd5 in mono_runtime_invoke
#17 0x007fa82b905ee2 in scripting_method_invoke(ScriptingMethodPtr, ScriptingObjectPtr, ScriptingArguments&, ScriptingExceptionPtr*, bool)
#18 0x007fa82b9042ba in ScriptingInvocation::Invoke(ScriptingExceptionPtr*, bool)
#19 0x007fa82b8ea397 in MonoBehaviour::CallUpdateMethod(int)
#20 0x007fa82b65538d in void BaseBehaviourManager::CommonUpdate<LateBehaviourManager>()
#21 0x007fa82b7c17ae in ExecutePlayerLoop(NativePlayerLoopSystem*)
#22 0x007fa82b7c17f1 in ExecutePlayerLoop(NativePlayerLoopSystem*)
#23 0x007fa82b7c1d2a in PlayerLoop()
#24 0x007fa82b95e004 in PlayerMain(int, char**)
#25 0x007fa82ac3f480 in __libc_init_first
#26 0x007fa82ac3f539 in __libc_start_main
#27 0x00000000400569 in _start

[pleroy]

I have stared at this code before, and I have wondered about this optional although from my analysis things looked correct. Apparently they are not. I cannot find the issue that I was investigating, but see #3882.

[pleroy]

@jenden0:

1. Do you have a crash folder? If so, please give it to us. (I'm not sure how things look on Linux, though.)
2. If you know how to reproduce the problem, could you give us a journal?

[pleroy]

I was able to reproduce the problem in a unit test.

There is a curious race in the construction of PushPullExecutor. The member variables of that class are declared as follows:

Principia/base/push_pull_callback.hpp

Lines 82 to 85 in e384167

	PushPullCallback<Result, Arguments...> callback_;
	std::thread thread_;
	mutable absl::Mutex lock_;
	std::optional<T> result_ GUARDED_BY(lock_);

and the constructor is as follows:

Principia/base/push_pull_callback_body.hpp

Lines 87 to 96 in e384167

	template<typename T, typename Result, typename... Arguments>
	PushPullExecutor<T, Result, Arguments...>::PushPullExecutor(Task task)
	: thread_([this, task = std::move(task)]() {
	auto const result = task(callback_.ToStdFunction());
	{
	absl::MutexLock l(&lock_);
	result_ = result;
	}
	callback_.Shutdown();
	}) {}

Note how the (implicit) construction of the member variables lock_ and result_ occurs after the construction of the member thread_. So if thread_ is executing "fast enough" it can assign a value to result_ at line 93 before the construction of that member variable has happened. Some time after the thread terminates, result_ would be reset to std::nullopt and the invariant of the class (if the thread has terminated, result_ contains a value) would be violated.

Now, can this happen? Can the thread execute so quickly that it finishes before result_ is constructed? In general, the execution of the task at line 90 is going to imply data exchange between C++ and C#, with the attendant synchronization, and therefore will take a long time. Except in one case, which is if task can detect that there is no collision (and return std::nullopt) without exchanging any data between C++ and C#. That would happen if for instance the trajectory is far enough from the centre of the celestial.

[jenden0]

The vessel in question was in a tundra orbit with a very high AP, so it's certainly possible that it qualified as "far enough from the centre of the celestial" for at least some part of the orbit.