search

FairRootGroup / FairRoot

C++ simulation, reconstruction and analysis framework for particle physics experiments
http://fairroot.gsi.de
Other
57 stars 96 forks source link

LOG(fatal) is supposed to throw an exception but it never throws because of abort from fclose(stderr) #1463

Open YanzhaoW opened 10 months ago

YanzhaoW commented 10 months ago

Describe the bug https://github.com/FairRootGroup/FairRoot/blob/fa7108b45684c729c2e88b12d7fbf4bbb5a4dc65/fairtools/FairLogger.cxx#L192-L203

Here the program somehow call the abort when calling fclose(stderr) in the if statement. Therefore, it never reaches to the throwing line.

To Reproduce Add a printout just above the throw statement and the printout message never gets printed.

Expected behavior Let it throw

rbx commented 9 months ago

A different idea: a LOG call should never throw (unless it can't actually log) because of a severity level. It's job is just to make a log entry. The way it is now is just historical, but it is not good - it is not loggers job to terminate your program. Deprecate/remove it.

If you really want to have a fatal call which throws, call it Fatal() or FATAL macro, which would internally first call LOG, and then throw.

YanzhaoW commented 7 months ago

Yeah, I agree. But would this be a huge breaking change? Many programs would not be terminated in the emergency, which could cause lots of segmentation fault.

rbx commented 7 months ago

I did a quick test with LOG(fatal) at (https://github.com/FairRootGroup/FairRoot/blob/dev/examples/MQ/serialization/data_generator/dataGenerator.cxx#L94) and it throws the exception as expected (which would itself lead to SIGABRT if uncaught).

Could you post a stacktrace to see a bit more detail? Also, what is the corefile filename and does it contain anything meaningful? Perhaps the failure happens there already.

I wonder why the close(stderr) is needed at all. Perhaps it is aborting because it is closed already at that point?