softIocPVA calls iocInit unrequested and then complains

[mdavidsaver]

Originally reported as https://bugs.launchpad.net/epics-base/+bug/1904207

@dirk-zimoch says:

Since R7.0.4.2, softIocPVA automatically calls iocInit after reading the startup script, even if iocInit had already been called in that script. In that case it complains:
iocBuild: IOC can only be initialized from uninitialized or stopped state

softIoc does not do this only softIocPVA does.

Please revert to the previous behavior which leaves the choice to the user if iocInit should be called or not.

@ttkorhonen says:

+1
This would break a lot of our applications and startup scripts.

[ralphlange]

Just happy that 7.0.4.2 is not out yet...

[mdavidsaver]

The most recent changes to pdbApp/softMain.cpp were 9499137, 8363c87, and e843db5.

@ttkorhonen Can you clarify "would break". What Dirk reports sounds like a nuisance message. Are you seeing something more serious?

@ralphlange These were all prior to 7.0.4.1

[mdavidsaver]

softIoc does not do this only softIocPVA does.

This doesn't track. As of 7.0.4.1 these two were in sync. The only subsequent change was to add -v flag to softIoc.

Can you give any specific examples of a script + CLI arguments which now produce warnings or failures?

[mdavidsaver]

The current logic does not check the return code of iocInit().

pva2pva/pdbApp/softMain.cpp

Lines 239 to 244 in 0e77203

	if (loadedDb) {
	if (verbose)
	std::cout<<"iocInit()\n";
	iocInit();
	epicsThreadSleep(0.2);
	}

Don't ask me about the sleep, that had been there for awhile. Probably residual paranoia.

[ttkorhonen]

Maybe "breaks" was not the correct expression but this would be a real nuisance. I use quite often the possibility to stop before iocInit and start manually from the shell. To be honest, I have not tested myself, I just reacted to Dirk's report.

[dirk-zimoch]

The message as such is "technically harmless". But not without impact. Users will come to me all the time and ask about that error message thinking they did something wrong.

What would "break" might be test setups which expect the ioc to be in an unfinished state so that they can add functionality from the command line (e.g. load more records) before calling iocInit.

[mdavidsaver]

What would "break" might be test setups which expect the ioc to be in an unfinished state

Right, but I don't think this has changed. The -S argument should still be respected. This is why I ask for an example or two. This might just be my lack of imagination about how softIoc* can be invoked.

[mdavidsaver]

oops, -S does something else. Still, I don't think the logic changed substantially.

[dirk-zimoch]

What about -S ? I do not want to prevent an interactive shell.

My point is: Before, the user had to write "iocInit" in the startup script. So everyone did it. Now the iocsh complains. To get rid of the message, all startupp scripts need to be changed. I would prefer not to have such unexpected changes of behavior.

I do not like programs that try to outsmart me. When call iocInit in my startup script, I mean it. If I don't call iocInit I mean it, too.

[dirk-zimoch]

A new command line option that makes softiocPVA call iocInit would be fine. But the default should be not to do it.
And softiocPVA should behave the same as softioc.

[mdavidsaver]

Base 3.15 still has the previous logic, where iocInit() is also called conditionally. (3.14 as well)

https://github.com/epics-base/epics-base/blob/c969f05f51841a4bd17daad37b09bd175d63fc7a/src/std/softIoc/softMain.cpp#L207-L210

    if (loadedDb) {
	iocInit();
	epicsThreadSleep(0.2);
    }

It is entirely possible that handling of the loadedDb flag changed, perhaps in unintended ways, when I reworked the CLI argument processing. A specific example would help me to understand if/how this is happening.

[dirk-zimoch]

Example file startup.cmd:

iocInit

Command: softIocPVA -D dbd/doftIocPVA.dbd startup.cmd

Before:

iocInit
Starting iocInit
############################################################################
## EPICS R7.0.4.1-DEV-2020-07-06
## Rev. PSI-7.0.4.1
############################################################################
iocRun: All initialization complete
epics> 

Now:

dbLoadDatabase("/usr/local/epics/base-7.0.4.1/dbd/softIocPVA.dbd")
softIocPVA_registerRecordDeviceDriver(pdbbase)
# Begin startup.cmd
iocInit
Starting iocInit
############################################################################
## EPICS R7.0.4.2-DEV-2020-11-13
## Rev. PSI-7.0.4.1-89-g81d139b7f786b6d6f4ba-dirty
############################################################################
iocRun: All initialization complete
# End startup.cmd
iocInit()
iocBuild: IOC can only be initialized from uninitialized or stopped state
epics> 

[dirk-zimoch]

Empty startup file: null.cmd

Command: softIocPVA null.cmd

Before:

epics>

Now:

dbLoadDatabase("/usr/local/epics/base-7.0.4.1/bin/SL6-x86_64/../../dbd/softIocPVA.dbd")
softIocPVA_registerRecordDeviceDriver(pdbbase)
# Begin null.cmd
# End null.cmd
iocInit()
Starting iocInit
############################################################################
## EPICS R7.0.4.2-DEV-2020-11-13
## Rev. PSI-7.0.4.1-89-g81d139b7f786b6d6f4ba-dirty
############################################################################
iocRun: All initialization complete
epics>

[dirk-zimoch]

Hm... I tested now with softIoc (without PVA), something that I had not done for a long fime for EPICS 7. Both, 7.0.4.1 and 7.0.4.2 call iocInit on their own. 3.14.12.7 does not do this.

[mdavidsaver]

Ok, I see what changed.

The order of the two conditions:

pva2pva/pdbApp/softMain.cpp

Lines 222 to 232 in cb13435

	if (loadedDb) {
	iocInit();
	epicsThreadSleep(0.2);
	}
	/* run user's startup script */
	if (argc>0) {
	if (iocsh(*argv)) epicsExit(EXIT_FAILURE);
	epicsThreadSleep(0.2);
	loadedDb = 1; /* Give it the benefit of the doubt... */
	}

Is reversed. So the "Give it the benefit of the doubt..." case is triggered.

pva2pva/pdbApp/softMain.cpp

Lines 224 to 244 in 0e77203

	if(optind<argc) {
	// run script
	// ignore any extra positional args (historical)
	if (verbose)
	std::cout<<"# Begin "<<argv[optind]<<"\n";
	errIf(iocsh(argv[optind]),
	std::string("Error in ")+argv[optind]);
	if (verbose)
	std::cout<<"# End "<<argv[optind]<<"\n";
	epicsThreadSleep(0.2);
	loadedDb = true; /* Give it the benefit of the doubt... */
	}
	if (loadedDb) {
	if (verbose)
	std::cout<<"iocInit()\n";
	iocInit();
	epicsThreadSleep(0.2);
	}

[mdavidsaver]

It looks to me like the cause of the (my) confusion is that loadedDb is also used to decide to call epicsThreadExitMain().

@anjohnson @ralphlange Do either of you know what is going on with epicsThreadExitMain(). Is it (still) needed by softIoc? Why isn't it called by main() in the iocApp template?

pva2pva/pdbApp/softMain.cpp

Lines 255 to 256 in 0e77203

	if (loadedDb) {
	epicsThreadExitMain();

[anjohnson]

This is the code at the end of main() in the original (3.14.x) version of softMain.cpp, I think it shows the intention pretty clearly:

    /* run user's startup script */
    if (argc>0) {
        if (iocsh(*argv)) epicsExit(EXIT_FAILURE);
        epicsThreadSleep(0.2);
        loadedDb = 1;   /* Give it the benefit of the doubt... */
    }

    /* start an interactive shell if it was requested */
    if (startIocsh) {
        iocsh(NULL);
    } else {
        if (loadedDb) {
            epicsThreadExitMain();
        } else {
            printf("%s: Nothing to do!\n", arg0);
            usage(EXIT_FAILURE);
        }
    }
    epicsExit(EXIT_SUCCESS);

If the user loaded at least one database and did not want an interactive iocsh the epicsThreadExitMain() call keeps the IOC running, otherwise it would immediately fall through to the epicsExit() and stop.

[anjohnson]

In case it's not obvious, the _APPNAME_Main.cpp template doesn't give the user the ability to not start an interactive iocsh, hence it has no need to call epicsThreadExitMain().

[mdavidsaver]

epicsThreadExitMain() call keeps the IOC running

hum. That may have been the intent, but isn't the case with the 3.15 or 7.0 branches at present. epicsThreadExitMain() is free()'ing the implicitly created epicsThreadOSD* for main(). Nothing about the posix version blocks. The WIN32 version simply calls _endthread, which seems roughly equivalent to pthread_exit().

And with 3.15 or 7.0, actually passing -S and an empty iocsh script seems to result in a half dead softIoc process which I have to SIGKILL!

So it seems like there are some problems with softIoc going back some time.

[mdavidsaver]

softIoc does not do this only softIocPVA does.

Since the logic problems turn out not to be specific to softIocPVA, I'm going to switch back to lp:1904207 (sorry for jumping over here too soon) and make changes softMain.cpp in Base first. Once working, those changes can be sync'd to pva2pva.

[anjohnson]

The purpose of the epicsThreadExitMain() routine was to provide a way for the main thread to exit but leave all of the other threads (and hence the IOC) running. It does actually still seem to work fine for me on the tip of the 7.0 branch:

tux% softIoc -x anj -S
dbLoadDatabase("/home/phoebus4/ANJ/epics/base/7.0/bin/linux-x86_64/../../dbd/softIoc.dbd")
softIoc_registerRecordDeviceDriver(pdbbase)
iocInit()
Starting iocInit
############################################################################
## EPICS R7.0.4.2-DEV
## Rev. R7.0.4.1-22-g3e891a12ff9b152620d1-dirty
############################################################################
iocRun: All initialization complete

To exit the IOC I do caput anj:exit 1 from another shell and my shell prompt appears.

[anjohnson]

Actually that wasn't quite the tip of the 7.0 branch but close enough, the tip still works but I don't get the verbose output any more:

tux% bin/linux-x86_64/softIoc -x anj -S
Starting iocInit
############################################################################
## EPICS R7.0.4.2-DEV
## Rev. R7.0.4.1-32-g8fd36d8eef3420b42e79-dirty
############################################################################
iocRun: All initialization complete

Note that the IOC doesn't attempt to detach from its parent thread so doing this in a shell results in something that you can't even Ctrl-C or Ctrl-Z out of. If you mean to background it, do that on the command-line.

[mdavidsaver]

Right, but is this really a reasonable way to accomplish this? eg. it seems wrong on windows at least, where return from main() will normally implicitly kills all other threads (cf. our discussions about epicsAtExit() handlers). Calling _endthread() from main() does not appear to be documented behavior. I guess it sort of works with Linux, but results in a strange situation where the PID for the process points to a ~zombie thread which eg. can't receive signals. This means I can't attach a debugger to it.

[mdavidsaver]

Anyway, since we can agree on the intent I'll replace epicsThreadMain() with a loop+epicsThreadSleep().

[anjohnson]

Why not fix the original bug first, preventing softIocPVA from calling iocInit() when it didn't used to. The epicsThreadExitMain() call might not do what you think it should, but it still does exactly the same thing it always has and that wasn't what was being complained about. We can discuss changing it as a separate matter.

[anjohnson]

s/didn't used to/softIoc doesnt/