-
Notifications
You must be signed in to change notification settings - Fork 298
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
ExternalMedia library, OMEdit crash with Test.fluidProp.WaterIF95.TestBasePropertiesDynamic #2838
Comments
Original ticket https://trac.openmodelica.org/OpenModelica/ticket/2838 AFAICT, this still happens on Windows with OM 1.20 and current ExternalMedia master (f524f94), binaries from https://github.com/modelica-3rdparty/ExternalMedia/suites/9592856454/artifacts/457340537 -- running the example crashes OM. |
@bilderbuchi I need to go for one more full rounds of ExternalMedia tests before finally making the releases. Too much stuff in the pipeline... |
😅 I know the feeling! No worries, I just used the opportunity of the 1.20 release to re-check issues, if they still reproduce. |
I just retested this with a recent OM nightly (1.21.0-dev-318) and current ExternalMedia master (modelica-3rdparty/ExternalMedia@db55653) with binaries from the CI job, and running Even running with
omc just silently quits when trying to simulate the model with it. Irrespective of progress in ExternalMedia, I think OM should not just crash when there is a problem with a package/external library, but catch the exception and emit an error message. |
Running the model in omc using gdb reveals a segfault that probably should be caught:
backtrace
|
@bilderbuchi thanks for reporting. Unfortunately, this stems from a basic issue, which is discussed in modelica/ModelicaSpecification#2191 ad #9029: there is currently no guaranteed way to access ModelicaError from externally compiled code that may be called either at run time or compile time, with all Modelica tools and with all major operating systems. Thanks to @fedetft we eventually managed to conjure up a strategy that works in most cases. It works well if the error is generated when actually computing medium properties at runtime, which is normally initiated by calling setState_XX functions. This is currently handled successfully with ModelicaError with Dymola and OMC under Linux and Windows. This means that if a solver goes beyond the validity limits of a certain medium models, and the external code throws an error, the exception is caught by the tool and the solver retries with a different step or something. Unfortunately, we couldn't find a way to have ModelicaError handled also at compile time. In order to do so, we would need all Modelica tools to export the ModelicaError symbol, not only in the executable simulation codes, but also in the tools themselves. This would allow to check if the solver exists and report an error in that case, instead of segfaulting. This is actually already possible with the Linux versions of OMC and Dymola (probably by sheer chance), but alas not for Windows versions. When you compile a model using ExternalMedia with OMC, the frontend evaluates all constants at compile time, and to do so it triggers a chain of events that leads to calling the some function in the externally compiled DLL at compile time already. That function calls SetFluid, which crashes because there is no pointer to the solver. Of course we could check if the pointer is null and report an error, but the problem is, we can't call ModelicaError in this context. In principle we could implement a workaround for this issue: if the solver does not exist, set the constants to some dummy value, and delay the generation of the proper error to the runtime, when setState_XX is called during initialization, and ModelicaError is available. However, this would pollute the code even more than it already is with low-level hacks which are difficult to understand, explain, and maintain, I'm not sure that is the best way to go. We needed the hacks to finally get the thing going, but enough of that already. I think we should instead invest our time into trying to convince the community to standardize and provide full support of ModelicaError (and in fact all Modelica.Utilities) functions both at run time and compile time without the need of intricate hacks. That would solve all problems at the root: when something wrong happens, you just call ModelicaError, and you don't have to worry if the symbols are available, or to pass around pointers to that function, or other low-level, tool-dependent stuff. It will also ensure that ExternalMedia runs out of the box on other Modelica tools we haven't tried out yet. Do you want to help us with that? If so, we can restart the discussion on modelica/ModelicaSpecification#2191. |
Ah, that sounds like gnarly problem. I had seen/skimmed #9029, but I'm quickly out of my depth with linking intricacies. I understand the problem I think and accept that it happens and this is unavoidable for the time being. What I don't fully get, as to me it seems quite avoidable, is that a segfault in the compiler crashes OMEdit. It's like you compile a malformed C++ program and your Visual Studio just closes -- quite unexpected and undesirable! Is there no equivalent to what I would do in Python to limit the blast radius of an Exception? try:
ret = compile(somecode)
except RuntimeError as exc: # or "Exception" if you want to catch all of them
print(f'Oopsie, compilation failed: {exc}!') I'm not sure how I could help with modelica/ModelicaSpecification#2191, as I'm by no means an expert. Also, before restarting another |
It is 😓
OMEdit is directly linked to OMC, so the communication is pretty low-level. I think in the past we used CORBA, which is of course safer, but slow as hell, eventually we needed something more direct.
Not sure, @adrpo, @mahge, @perost, @sjoelund any idea?
You can help pushing as a developer for the need of calling utilities functions in all contexts.
I lost hope on that. At least, I would not put it on the critical path 😅 |
Python is an interpreter but if you call some external C library that will dereference a NULL pointer Python will die very nicely. There is no exception handling for that. It might be possible to set some event handler and try to recover but sometimes is impossible to recover. |
In December, I came up with a possible solution for the stumbling block that I'm still hoping for your feedback/comments on. I planned to wait until after 1.21 before bumping you again, though, as I can see how busy you are. :-) |
Ah, I was not aware, I assumed this was ZMQ (or something); I can see how being linked as a library will complicate things. 😅 |
It's not really safe to catch segmentation faults. Anything could have happened in the code with dangling pointers/etc. |
We reached a point where it makes no sense to try adding further workarounds for a problem that has no solution. Catching segmentation faults and recovering in the general case is impossible. There are already enough workarounds in ExternalMedia that keep failing in certain corner cases. I think we need to address the root cause of the problem by forcing the openmodelica specifications to make error reporting functions available both at compile time and at run-time. The possibility to call external functions at compile time to resolve package constants coupled with the lack of mandated, standardized error reporting at compile time results in a broken language specification. Period. |
No description provided.
The text was updated successfully, but these errors were encountered: