Fix the bug introduced by the PR #1406.

[bcoconni]

Following the merge of PR #1406, the test TestFunctions.py fails with a SEGFAULT.

TLDR: The failure is due to trying to run Python code while a Python exception has been raised. This PR catches the Python exception prior to running code in the FGLogger class then rethrows.

The failure sequence

The test test_table_exceptionin TestFunctions.py sets the conditions for FGFDMExec.load_model to throw an exception then checks that the exception has indeed been thrown:

jsbsim/tests/TestFunctions.py

Lines 330 to 331 in fc8156d

	with self.assertRaises(BaseError):
	tripod.start()

The execution of this test triggers the following exception from FGTable.cpp:

jsbsim/src/math/FGTable.cpp

Lines 126 to 130 in fc8156d

	} else if (!call_type.empty()) {
	XMLLogException err(el);
	err <<" An unknown table type attribute is listed: " << call_type << "\n";
	throw err;
	}

In the Python binding, the method FGFDMExec::LoadModel is set to use the exception handler from the function convertJSBSimToPyExc:

jsbsim/python/jsbsim.pxd

Lines 218 to 219 in fc8156d

	bool LoadModel(string model,
	bool add_model_to_path) except +convertJSBSimToPyExc

Since there is no catch instruction for the LogException type in convertJSBSimToPyExc, the C++ exception is "converted" to a Python exception BaseError:

jsbsim/python/src/ExceptionManagement.h

Lines 53 to 55 in fc8156d

	catch (const JSBSim::BaseException& e) {
	PyErr_SetString(base_error, e.what());
	}

THEN the C++ exception instance e is destroyed which triggers the call to the destructor BufferLogger which sets the logging level of the FGLogger instance stored in GlobalLogger (i.e. calls GlobalLogger::SetLevel):

jsbsim/src/input_output/FGLog.cpp

Lines 116 to 120 in fc8156d

	BufferLogger::~BufferLogger()
	{
	if (tokens.empty()) return;
	GlobalLogger->SetLevel(log_level);

For that test, the logger is an instance of PyLogger, so PyLogger::SetLevel is called:

jsbsim/python/src/PyLogger.cxx

Lines 73 to 79 in fc8156d

	void PyLogger::SetLevel(LogLevel level) {
	const int idx = static_cast<int>(level);
	assert(idx >=0 && idx <= 6);
	PyObjectPtr py_level = convert_level_enums[idx];
	PyObjectPtr result = CallPythonMethodWithArguments("set_level", py_level);
	if (result) FGLogger::SetLevel(level);
	}

And this is where the Python interpreter terminates the program since the Python method set_level is called while an exception has been raised by the earlier call to PyErr_SetString in ExceptionManagement.h.

The fix

The idea is to catch the Python exception, to run the Python method set_level then to re-throw the Python exception.

For that purpose, the PR adds a new class PyExceptionHandler that catches and re-throws Python exceptions using RAII:

• The constructor catches the Python Exception - if any - using PyErr_Fetch.
• The destructor re-throws the exception - if any - using PyErr_Restore. The destructor also manages the case where the execution of set_level would raise an exception. In that case, the new exception is considered "unraisable": it is written in stderr then discarded by PyErr_WriteUnraisable. (However PyErr_WriteUnraisable can be "intercepted" by sys.unraisablehook if one needs to detect their occurrence - see this blog post from the author of that feature).

With the new class PyExceptionHandler:

• The occurrence of a Python exception (of any type) is caught
• If no exception was raised or if the exception is of type LogException then the code proceed with set_level otherwise it does nothing.
• When the destructor of PyExceptionHandler is executed, the exception is re-thrown (whatever the exception was), or nothing is done if no exception was raised.
• Repeat for each method of the Python class FGLogger.

Note that I have created a new Python exception LogExceptionError to be able to distinguish occurrences of JSBSim::BaseException from occurrences of JSBSim::LogException. The method set_level is only executed when an occurrence of the latter is raised.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 25.12%. Comparing base (fc8156d) to head (a5ea2d3).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1413   +/-   ##
=======================================
  Coverage   25.12%   25.12%           
=======================================
  Files         169      169           
  Lines       18721    18721           
=======================================
  Hits         4703     4703           
  Misses      14018    14018           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[agodemar]

@bcoconni great contribution, looks good to me.

[seanmcleod70]

Looks good.

[bcoconni]

OK. Thanks. PR merged.