Code review comment for lp:~epics-core/epics-base/ioc-shutdown

mdavidsaver (mdavidsaver) wrote :

On 12/03/2013 02:35 PM, Andrew Johnson wrote:
> For future reference: With almost 2**10 lines of diff this branch mixes together (admittedly related) changes to several different subsystems,

??? The merge proposal shows:

961 lines (+337/-53) 21 files modified

Also, the branch consists of many small, focused, changesets.

> The documentation branch does not cover all the changes made here, it only talks about the modifications to the callback subsystem.

Agreed, this needs to change.

> Is it wise to provide public APIs to all these Shutdown functions?

I would like to wrap all of this up into a single function (say
"testdbCleanup"). We could then make private header(s) for the many
shutdown calls. Would this make the change more acceptable.

> Could we shut everything down by calling epicsExitCallAtExits() instead, and just ensure that each system's registered shutdown function leaves the subsystem in a state that can be properly restarted?

This is an option I suppose.

> The idea of resetting OnceFlags makes me cringe;

A good point.

> I'm not convinced that restarting an IOC inside the same process is a good idea, given the code we have (I know you're trying to change that, but I think it needs a properly planned solution, which this isn't [yet]). Just making the shutdown APIs available will mean that someone will want/try to use them on a running hard IOC, which is not going to work since we can't communicate the shutdown request to any device support or driver threads (other than by calling their epicsExitCallAtExits() routines, but existing support don't use that to prepare themselves for a restart).

The goal is unittests. It it would help in your mind we can only allow
shutdown/cleanup between calling testPlan() and testDone().

« Back to merge proposal