EPICS Base

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

ioc-shutdown
Merge into 3.15

Revision history for this message

Andrew Johnson (anj) wrote on 2013-12-03:

On 12/03/2013 03:39 PM, 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

961 ≈ 1024

I'm just saying that the longer the diff is, the more reviewers will be
put off looking at it. If the changes were split into smaller dependent
branches, reviewing them might look like less work and could be done in
smaller chunks (this was a human psychology comment, not a programming
issue at all).

>> 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.

Hmm, maybe. How about an alternative API to iocInit, for test programs
only, and implemented outside of iocInit.c? I would be quite happy for
the various init*Sup() and related routines to be moved out of there and
into ioc/db somewhere, dbInit.c maybe?

>> 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 advantage of using epicsExitCallAtExits() is that we don't have to
expose the various Shutdown symbols at all, and we automatically stop
everything that we've started, in the (correct) reverse order.

The disadvantage is this will slow down IOC shutdown unnecessarily,
since we only need the 'clean up ready to start up again' actions in
those exit routines when we're running tests that need restart.

I'm getting visions of a subsystem management API; subsystems would
register both a fast exit and a slow clean-up routine with it. It could
even incorporate the taskwd, and provide subsystem/task status & health
info on request. This could also somewhere to centralize all of the
init/run/pause/exit enum task controls that are proliferating (which I
agree should probably become atomics).

Too much work?

- Andrew
--
Advertising may be described as the science of arresting the human
intelligence long enough to get money from it. -- Stephen Leacock

961 ≈ 1024

I'm just saying that the longer the diff is, the more reviewers will be 
put off looking at it. If the changes were split into smaller dependent 
branches, reviewing them might look like less work and could be done in 
smaller chunks (this was a human psychology comment, not a programming 
issue at all).

>> 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.

Hmm, maybe. How about an alternative API to iocInit, for test programs 
only, and implemented outside of iocInit.c? I would be quite happy for 
the various init*Sup() and related routines to be moved out of there and 
into ioc/db somewhere, dbInit.c maybe?

>> 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 advantage of using epicsExitCallAtExits() is that we don't have to 
expose the various Shutdown symbols at all, and we automatically stop 
everything that we've started, in the (correct) reverse order.

The disadvantage is this will slow down IOC shutdown unnecessarily, 
since we only need the 'clean up ready to start up again' actions in 
those exit routines when we're running tests that need restart.

I'm getting visions of a subsystem management API; subsystems would 
register both a fast exit and a slow clean-up routine with it. It could 
even incorporate the taskwd, and provide subsystem/task status & health 
info on request. This could also somewhere to centralize all of the 
init/run/pause/exit enum task controls that are proliferating (which I 
agree should probably become atomics).

Too much work?

- Andrew
-- 
Advertising may be described as the science of arresting the human
intelligence long enough to get money from it. -- Stephen Leacock

« Back to merge proposal