Re: How to safely implement an asynPortDriver with SCAN records

[Michael Davidsaver via Tech-talk <tech-talk at aps.anl.gov>]

On 7/8/22 11:11, Mark Rivers via Tech-talk wrote:
> Hi Andrew,
>
> To be honest I have not paid enough attention to how to correctly handle destructors in asynPortDriver.

On the occasions when I've used asynPortDriver, I put a cantProceed() in the sub-class destructor.

After a call to registerInterface(), the asynInterface::drvPvt pointer must remain valid until
the process exits.  So an asyn port can't be destroyed.


I've toyed with the ideas like re-counter asynUser, or somehow later setting drvPvt=NULL.
Both seem possible, but not without quite some effort.


This situation is made more complicated by the fact that IOC scan threads are not stopped on exit.
We do now stop them during database unit tests.  However, some ~vague compatibility concerns have
so far kept this from being done for full IOCs.


Personally, as a matter of good practice, drivers I write always include an epicsAtExit() handler
which stops any worker threads.


Also, I should mention the de-init hooks added in Base 7.0.4 to decouple database shutdown from
process exit.

https://github.com/epics-base/epics-base/commit/5d5e552a7ec6ef69459c97b0081aa775372a6290#diff-02e4602d43dbb03b4097bb26382a1db92367fb55fe791415cd32d4f234875d92R50

For compatibility, the effect of initHookAtShutdown can mostly be replicated by calling epicsAtExit()
at initHookAfterIocRunning.


> This is one of Erico’s tracebacks.
>
> ==2685== Invalid read of size 4
>
> ==2685==    at 0x7BFA904: __pthread_mutex_lock_full (pthread_mutex_lock.c:176)
>
> ==2685==    by 0x5E92CA5: epicsMutexOsdLock (in /opt/epics/base-3.15.9/lib/linux-x86_64/libCom.so.3.15.9)
>
> ==2685==    by 0x5093A5F: asynPortDriver::lock() (in /opt/epics/synApps-lnls-R1-2-1/support/asyn-R4-35/lib/linux-x86_64/libasyn.so)
>
> ==2685==    by 0x50927BB: readUInt32Digital (in /opt/epics/synApps-lnls-R1-2-1/support/asyn-R4-35/lib/linux-x86_64/libasyn.so)
>
> ==2685==    by 0x50ABF79: processCallbackInput (in /opt/epics/synApps-lnls-R1-2-1/support/asyn-R4-35/lib/linux-x86_64/libasyn.so)
>
> ==2685==    by 0x5071EE2: portThread (in /opt/epics/synApps-lnls-R1-2-1/support/asyn-R4-35/lib/linux-x86_64/libasyn.so)
>
> ==2685==    by 0x5E90A9B: start_routine (in /opt/epics/base-3.15.9/lib/linux-x86_64/libCom.so.3.15.9)
>
> ==2685==    by 0x7BFCEA4: start_thread (pthread_create.c:307)
>
> ==2685==    by 0x90EBB0C: clone (clone.S:111)
>
> ==2685==  Address 0xe8b7b20 is 16 bytes inside a block of size 48 free'd
>
> ==2685==    at 0x4C2B06D: free (vg_replace_malloc.c:540)
>
> ==2685==    by 0x5E8BB55: epicsMutexDestroy (in /opt/epics/base-3.15.9/lib/linux-x86_64/libCom.so.3.15.9)
>
> ==2685==    by 0x5094D76: asynPortDriver::~asynPortDriver() (in /opt/epics/synApps-lnls-R1-2-1/support/asyn-R4-35/lib/linux-x86_64/libasyn.so)
>
> ==2685==    by 0x5E86FB2: epicsExitCallAtExits (in /opt/epics/base-3.15.9/lib/linux-x86_64/libCom.so.3.15.9)
>
> ==2685==    by 0x5E87357: epicsExit (in /opt/epics/base-3.15.9/lib/linux-x86_64/libCom.so.3.15.9)
>
> ==2685==    by 0x40786C: main (in /opt/epics/ioc/fofb-epics-ioc/bin/linux-x86_64/FOFB)
>
> ==2685==  Block was alloc'd at
>
> ==2685==    at 0x4C2C089: calloc (vg_replace_malloc.c:762)
>
> ==2685==    by 0x5E92B24: epicsMutexOsdCreate (in /opt/epics/base-3.15.9/lib/linux-x86_64/libCom.so.3.15.9)
>
> ==2685==    by 0x5E8B98E: epicsMutexOsiCreate (in /opt/epics/base-3.15.9/lib/linux-x86_64/libCom.so.3.15.9)
>
> ==2685==    by 0x5097584: asynPortDriver::initialize(char const*, int, int, int, int, int, int, int) (in /opt/epics/synApps-lnls-R1-2-1/support/asyn-R4-35/lib/linux-x86_64/libasyn.so)
>
> ==2685==    by 0x5098713: asynPortDriver::asynPortDriver(char const*, int, int, int, int, int, int, int) (in /opt/epics/synApps-lnls-R1-2-1/support/asyn-R4-35/lib/linux-x86_64/libasyn.so)
>
> ==2685==    by 0x63C80F3: asynNDArrayDriver::asynNDArrayDriver(char const*, int, int, unsigned long, int, int, int, int, int, int) (in /opt/epics/synApps-lnls-R1-2-1/support/areaDetector-R3-6/ADCore/lib/linux-x86_64/libADBase.so)
>
> ==2685==    by 0x4E4A02C: drvFOFB::drvFOFB(char const*, char const*, int, char const*, int, int, int, int, unsigned long) (in /opt/epics/ioc/fofb-epics-ioc/lib/linux-x86_64/libFOFBSupport.so)
>
> ==2685==    by 0x4E4C58B: drvFOFBConfigure (in /opt/epics/ioc/fofb-epics-ioc/lib/linux-x86_64/libFOFBSupport.so)
>
> ==2685==    by 0x4E4C5E7: initCallFunc (in /opt/epics/ioc/fofb-epics-ioc/lib/linux-x86_64/libFOFBSupport.so)
>
> ==2685==    by 0x5E81B9F: iocshBody (in /opt/epics/base-3.15.9/lib/linux-x86_64/libCom.so.3.15.9)
>
> ==2685==    by 0x407851: main (in /opt/epics/ioc/fofb-epics-ioc/bin/linux-x86_64/FOFB)
>
> So asynPortDriver created an epicsMutex. Erico’s destructor deleted itself and the base asynPortDriver, which deleted the epicsMutex.  But then processCallbackInput (from a scan task?) called readUInt32Digital, which tried to call the now destroyed asynPortDriver object.
>
> How is this supposed to work?  I believe he registered epicsAtExit in the dynamic constructor for his class that was called from st.cmd.  I think that is before the scan tasks are created if it is done before iocInit, correct? So it should be deleted after the scan tasks?  Is the problem that the port thread created by asynManager has a queue that is calling processCallbackInput, and that is still running even if the scan task has been deleted?
>
> Mark
>
> *From:* Andrew Johnson <anj at anl.gov>
> *Sent:* Friday, July 8, 2022 12:43 PM
> *To:* Érico Nogueira Rolim <erico.rolim at lnls.br>; Mark Rivers <rivers at cars.uchicago.edu>
> *Cc:* tech-talk <tech-talk at aps.anl.gov>
> *Subject:* Re: How to safely implement an asynPortDriver with SCAN records
>
> On 7/8/22 12:27 PM, Érico Nogueira Rolim via Tech-talk wrote:
>
>     Thank you for the quick answer!
>
>     On 08/07/2022 12:23, Mark Rivers wrote:
>
>         Hi Érico,
>
>         The asynPort driver base class does not have an exit handler to destroy itself when the IOC is exiting, that is left to derived classes.  Does your derived class establish an exit handler with epicsAtExit?
>
>         What leads you to believe that the asynPortDriver destructor is being called?
>
>     I'm sorry for the confusion, I meant to say that the destructor for our class that's derived from asynPortDriver is called. And indeed, we have its destructor registered with epicsAtExit (a bit ugly):
>
>     static void exitHandlerC(void *pPvt)
>     {
>          drvFOFB *pdrvFOFB = (drvFOFB *)pPvt;
>          pdrvFOFB->~drvFOFB();
>     }
>
>     We do this because the destructor is responsible for closing quite a few resources before we exit. I'm open to suggestions on better ways to do this.
>
>
> Not sure if this is relevant, but when do you register that exit handler? The registered epicsAtExit() routines are called in the reverse order in which they were added, so to ensure that your shutdown routines are called /before/ any that the IOC or asyn registers you should register yours /afterwards/. Calling epicsAtExit() from a static C++ constructor is likely to cause these kinds of problems, if that's where you put it try moving that to a routine which gets run from a command in your st.cmd file (in general a driver that needs one should register an exit routine for each device as it gets created by the associated st.cmd command).
>
> HTH,
>
> - Andrew
>
>
> --
>
> Complexity comes for free, Simplicity you have to work for.