Playing well with REPLs

[bcoconni]

JSBSim has traditionally been employed within monolithic applications (such as FlightGear), where any JSBSim error generally resulted in the termination of the entire program. This assumption has simplified error handling within JSBSim itself: errors were considered unrecoverable since the calling application was expected to terminate when they occurred.

These days are gone now that we are providing a Python module and a Matlab S-function. This has opened up new possibilities for using JSBSim in interactive Read-Eval-Print-Loop (REPL) environments. This shift necessitates a reassessment and upgrade of our error handling and object state management strategies.

Specifically, this issue aims to discuss the following key requirements for robust JSBSim usage within REPLs (including, but not limited to, Python and Matlab):

1. REPL Stability: Under no circumstances should JSBSim cause the REPL to crash or terminate unexpectedly. Sudden REPL terminations are extremely frustrating for users, as it can lead to the loss of unsaved work and create a perception of poor software quality.
2. Robust Object State Management: Users working in a REPL environment often engage in exploratory, trial-and-error workflows. JSBSim objects may therefore exist in various intermediate or inconsistent states due to user errors. JSBSim should handle these states gracefully, allowing users to recover and continue their work without requiring, for example, the complete destruction and recreation of their instance of FGFDMExec. This implies the need for mechanisms to:
   • Detect and report errors clearly and informatively.
   • Provide methods for users to query the state of an object?
   • Offer options to reset or correct invalid object configurations, such as the ability to call FGFDMExec::Load again after a previous failure.
   • Prevent operations that would lead to crashes or undefined behavior when an object is in an invalid state. Typically prevent executing FGFDMExec::Run if FGFDMExec::Load has failed.

To address these requirements, the following work areas should be addressed:

1. Memory Management: Implement more rigorous memory management practices to prevent leaks, especially when the execution is leaving the happy path (i.e. the ideal execution flow of a program, where all conditions are met, and no errors occur).
   • For example, pull requests Fix memory leaks in FGSwitch #1230 and Memory leak fix and general improvements of FGDistributor #1245 addressed memory leaks that occurred during error conditions, which was previously not a concern since the operating system would automatically reclaim all reserved memory upon program termination.
   • The straightforward way to address this requirement is to replace all the calls to new and delete by smart pointers (i.e. unique_ptr and shared_ptr).
2. Error Reporting: Ensure that error messages are clear, informative, and provide users with a clear understanding of the severity of the error. There should be no silently ignored or handled errors.
   • When JSBSim makes assumptions to address an error and continue execution, it should provide a message explaining the corrective action taken.
   • The on-going work on the new logging system is also addressing that point by adding a severity level (i.e. LogLevel) to each error message.
3. Exception Handling: Implement robust exception handling, ensuring that exceptions are caught where appropriate, or converted to a REPL-compatible error format (e.g., converting C++ exceptions to Python exceptions).

This issue serves as a starting point for discussing the necessary modifications and best practices to ensure that JSBSim can be used effectively and reliably in interactive environments. Future pull requests that contribute to this objective should reference this issue, to maintain a clear connection to the requirements and goals outlined here.

[bcoconni]

The straightforward way to address this requirement is to replace all the calls to new and delete by smart pointers (i.e. unique_ptr and shared_ptr).

Due to the design of JSBSim, this task is trivial in most cases and can be made by a newcomer to JSBSim so I'm flagging this issue as a good first issue.

[XueSongTap]

Can I work on this？

[bcoconni]

@XueSongTap, yes sure !

Just make sure to avoid submitting a huge PR with all the classes migrated to smart pointers at once. I'd rather suggest to start with a PR that just contains the changes for 1 or 2 classes and start the discussion from there.

I don't know how familiar you are with JSBSim code but FYI JSBSim is designed to allocate all its memory during initialization which is done in 2 steps:

1. General initialization where all the classes inheriting FGModel are instantiated.
2. Loading of the XML files where the allocated memory depends on the content of the XML files.

While running the simulation loop, JSBSim does not allocate nor release any memory.

All the memory is released when the object FGFDMExec is deleted.

[agodemar]

That's a great idea @bcoconni - I also envision interesting use cases in Julia within Jupyter notebooks.

[XueSongTap]

I'd like to contribute to this issue by starting with migrating a couple of components to use smart pointers. Based on the repository structure and the recommendations in this thread, I plan to work on:

models/flight_control/FGGain.cpp/FGGain.h
models/flight_control/FGSummer.cpp/FGSummer.h
These seem like relatively simple control system components that would make a good starting point. I believe they're fairly self-contained and would help establish patterns that could be applied to other components.

Since this would be my first contribution to JSBSim, I'd appreciate any guidance or feedback. Could someone from the team confirm if these would be appropriate components to begin with?

Thank you!

[bcoconni]

I plan to work on:

models/flight_control/FGGain.cpp/FGGain.h
models/flight_control/FGSummer.cpp/FGSummer.h

I agree FGGain.h is a good start but FGSummer.h does not allocate memory dynamically so there is nothing to change there.

In FGGain.h, you 'll notice that the type of Gain is FGParameter_ptr:

jsbsim/src/models/flight_control/FGGain.h

Lines 225 to 227 in e1ea4e8

	private:
	FGTable* Table;
	FGParameter_ptr Gain;

And FGParameter_ptr is a typedef for an home-made shared pointer SGSharedPtr.

jsbsim/src/math/FGParameter.h

Line 72 in e1ea4e8

typedef SGSharedPtr<FGParameter> FGParameter_ptr;

We might consider replacing the pointers using SGSharedPtr later with C++ standard smart pointers but for the moment, I suggest we leave them aside since they already are smart pointers.

Since this would be my first contribution to JSBSim, I'd appreciate any guidance or feedback. Could someone from the team confirm if these would be appropriate components to begin with?

Sure, I suggest you submit a PR with your proposal and we'll discuss your code from there.

[bcoconni]

Some further thoughts about exceptions in JSBSim.

I have this intuition that we should only throw exceptions when reading the XML files i.e. when calling the method FGModel::Load() and its siblings. In this context, throwing an exception means that JSBSim cannot load the data and aborts. That's OK, the user is expected to fix the problem and try again. For that scenario, we still need to add a try/catch in FGFDMExec::LoadScript and FGModel::LoadModel to clean everything up before returning but basically we are managing.

But what about exceptions thrown during the simulation loop, such as the one below ?

jsbsim/src/models/FGPropagate.cpp

Lines 360 to 363 in e1ea4e8

	case eBuss1:
	case eBuss2:
	case eLocalLinearization:
	throw("Can only use Buss (1 & 2) or local linearization integration methods in for rotational position!");

Here the simulation loop will abort and leave JSBSim in an unpredictable state. Hopefully, the caller has an exception handler or it will be terminated by the OS. And even so, assuming the user will fix the property simulation/integrator/position/translational (or whatever property was improperly set), I am very skeptical that the simulation could be resumed.

For the particular case of this exception, I think that the error (setting the property to an illegal value) should be intercepted earlier i.e. at the very moment the property is modified, when an error message could be issued and the property left to its current legal value which would allow the simulation to continue. So we should be able to avoid throwing an exception in this case.

Generally speaking, the idea is to use exceptions only during loading and avoid using them otherwise. I am not sure this policy is reasonable or even applicable to JSBSim but it makes sense to me.

Thoughts ?

[gallonmate]

That makes sense to keep exceptions for XML loading.

Expanding on your idea. I noticed logging is configurable to different levels. So what if exceptions are made configurable to different options as well? And maybe having either a delegate, a function, variable, or a class, etc - basically some way that can universally be handled by the REPLs as a way to gracefully handle JSBSim errors/exceptions.

For example if someone is running a standalone instance of JSBSim, it defaults to a basic level of exceptions, throwing the exceptions where it makes sense to. And then when JSBSim is used with REPLs or attached/embedded/etc with other programs, those users can select their desired level of exception. So that they can turn off JSBSim exceptions completely, and in place off JSBSim exceptions, then a delegate or some other means is called, so the REPLs/other programs can see there is an JSBSim error/exception and halt their simulation gracefully without crashing everything.

[bcoconni]

[...] basically some way that can universally be handled by the REPLs as a way to gracefully handle JSBSim errors/exceptions.

Maybe I'm misunderstanding your point but we are already doing that, at least for some methods of the Python classes: C++ exceptions are converted to Python's.

jsbsim/python/ExceptionManagement.h

Lines 33 to 57 in e1ea4e8

	void convertJSBSimToPyExc()
	{
	try {
	if (!PyErr_Occurred())
	throw;
	}
	catch (const JSBSim::TrimFailureException& e) {
	PyErr_SetString(trimfailure_error, e.what());
	}
	catch (const GeographicLib::GeographicErr& e) {
	PyErr_SetString(geographic_error, e.what());
	}
	catch (const JSBSim::BaseException& e) {
	PyErr_SetString(base_error, e.what());
	}
	catch (const JSBSim::FloatingPointException& e) {
	PyErr_SetString(e.getPyExc(), e.what());
	}
	catch (const std::string &msg) {
	PyErr_SetString(PyExc_RuntimeError, msg.c_str());
	}
	catch (const char* msg) {
	PyErr_SetString(PyExc_RuntimeError, msg);
	}
	}

For example if someone is running a standalone instance of JSBSim, it defaults to a basic level of exceptions, throwing the exceptions where it makes sense to. And then when JSBSim is used with REPLs or attached/embedded/etc with other programs, those users can select their desired level of exception. So that they can turn off JSBSim exceptions completely, and in place off JSBSim exceptions, then a delegate or some other means is called, so the REPLs/other programs can see there is an JSBSim error/exception and halt their simulation gracefully without crashing everything.

Well everything is possible, for that we could replace the keyword throw by a function which would trigger throw depending on some parameter. However the question is: what should we do if the user decide the exception should be ignored ? We cannot throw but we cannot continue the simulation either (otherwise we wouldn't need to throw).