Flush and close log files if they are still open at Logger destruction #14
Conversation
This allows all data that has been received by the logger to be orderly written out and the log file being closed properly, if the task is not moved through STOPPED before being destroyed.
| if(m_io) | ||
| m_io->flush(); | ||
| delete m_file; | ||
| delete m_io; |
There was a problem hiding this comment.
I don't mind, but that's already done in stopHook. You're essentially guarding against a major bug in RTT.
There was a problem hiding this comment.
Hmm. Interesting; i seem to remember having instrumented Logger::~Logger and Logger::stopHook to output a message and sometimes not seeing the one from the stopHook. I'll check again; maybe someone is instantiating Logger separately, maybe RTT has some problem in one of the shutdown paths.
There was a problem hiding this comment.
ORO_main/ORO_main_impl owns the deployments logger. If the deployment is terminated using SIGINT or so, Logger::stopHook is not called, even if the Logger has been started, but Logger::~Logger is.
I don't see where ORO_main would do anything with the logger after entering its wait loop(oro_thread), so the stopHook call would have to happen through the destruction of some of the objects at the end of ORO_main.
So, i went back to poking around inside RTT. The TaskContext::~TaskContext explicitly refuses to call into the stop() machinery. ExecutionEngine knows about it's task, but does nothing in its finalize() and only the exception paths potentially lead to the stopHook. The Activities seem to be more concerned with stopping their threads than stopping tasks, but they do call the ExecutionEngines finalize().
Don't get me wrong, i would have expected that the state machine is followed as much as possible. Maybe you have a better idea what path should have lead to stopHook being called?
There was a problem hiding this comment.
I actually believed the Deinitializer object in the deployment main would stop/deconfigure the tasks. You'd have to SIGKILL if you want to terminate the task without cleanup.
You can't call hooks from the destructors, they are virtual methods.
There was a problem hiding this comment.
The Deinitializer ends up only stopping activities(and avahi stuff), and those only call the empty ExecutionEngine::finalize(). Maybe the Deinitializer needs to be extended to also handle tasks?
There was a problem hiding this comment.
I think it should.
In this case, your patch makes sense no matter what (delete the pointers in destructor). But we really shouldn't have to replicate stopHook/cleanupHook in the destructor.
There was a problem hiding this comment.
Sure. I mean, i would have pushed this through just on grounds of it being defensive programming, but getting to the bottom of the underlying problem seemed important.
I created an issue over at orogen with the findings from here. I cannot promise to be able to work on this in a timely manner; may take a few months.
This allows all data that has been received by the logger to be orderly written out and the log file being closed properly, if the task is not moved through STOPPED before being destroyed.