Re: devEtherIP

["Kasemir, Kay" <kasemirk@ornl.gov>]

Hello Jeff:

That is a good discovery!

The driver is started when the device support's "init" routine is called
with 'after'=1, to which the app. dev. guide sais:
"This routine is called ... (with 'after'=1) ... after all records are
initialized but before the scan tasks are started".

The issue is that this is the last possible initialization step that a
driver has. The onceQ is initialized in initOnce(), which in turn is called
by scanInit(), which in turn happens _after_ the init(after=1) call to
device support.
For those records that are "Passive" and effectively scanned by changes on
the PLC, I see no better way than what you did to check if maybe the
database isn't ready, yet.

I'll include your changes, inc' the version numbers etc.

Thanks,
Kay 



On 11/3/10 16:55 , "Jeff Hill" <johill@lanl.gov> wrote:

> All,
> 
> Sometime back I mentioned that LANSCE was experiencing a crash
> in the Allen Bradley TCP/IP PLC driver (devEtherIP). There were some
> maintenance days this week, and I found some time to track down the
> cause. Apparently the following race condition is occurring.
> 
> o iocInit begins
> o device support initialization functions start the drvEtherIP
> auxiliary threads (one for each plc)
> o when a network message comes back from the plc the drvEtherIP
> auxiliary thread calls functions in devEtherIP which call scanOnce.
> o scanOnce tries to use the uninitialized "onceQ" ring buffer,
> and crashes.
> o later, iocInit calls initOnce
> o initOnce creates the "onceQ" ring buffer
> 
> We are experience the bug in R3.13, but looking at the latest
> sourceforge sources I am inclined to suspect that there is also
> a race condition vulnerability when the latest runs under R3.14.
> 
> Below are the patches I made to the driver. Unfortunately, it was
> necessary to create two versions; one for R3.13 and one for R3.14.
> We only run this driver on R3.13 IOC's so I have _not_ tested the R3.14
> version of the fix; Furthermore, the authors of this driver should
> review my fix as they may arrive at a better alternative.
> 
> Index: devEtherIP.c
> ===================================================================
> RCS file: /epics/cvs/extensions/ab5550/dev/devEtherIP.c,v
> retrieving revision 1.8
> diff -r1.8 devEtherIP.c
> 286a287,302
>> static void etherIP_scanOnce ( void * pRec )
>> {
>>      /*
>>       * astonished to discover that initHookRegister is in
>>       * initHooks.h in R3.13, but initHookRegister isnt
>>       * in iocCore object file in R3.13, so we resort
>>       * to an archaic brute force approach
>>       */
>> #    ifndef HAVE_314_API
>>      while ( ! interruptAccept ) {
>>          epicsThreadSleep  ( 0.1 );
>>      }
>> #    endif
>>      scanOnce ( pRec );
>> }
>> 
> 417c433
> <         scanOnce(rec);
> ---
>>       etherIP_scanOnce ( rec );
> 469c485
> <         scanOnce(rec);
> ---
>>       etherIP_scanOnce ( rec );
> 539c555
> <         scanOnce (rec);
> ---
>>       etherIP_scanOnce ( rec );
> 589c605
> <         scanOnce(rec);
> ---
>>       etherIP_scanOnce ( rec );
> Index: drvEtherIP.c
> ===================================================================
> RCS file: /epics/cvs/extensions/ab5550/dev/drvEtherIP.c,v
> retrieving revision 1.6
> diff -r1.6 drvEtherIP.c
> 25a26,27
>> #include "initHooks.h"
>> 
> 1295a1298,1301
>> #ifdef HAVE_314_API
>> static int databaseIsReady = false;
>> #endif
>> 
> 1308a1315,1320
>> #ifdef HAVE_314_API
>>     if (!databaseIsReady) {
>>        epicsMutexUnlock(drvEtherIP_private.lock);
>>        return 0;
>>     }
>> #endif
> 1388a1401,1430
>> #ifdef HAVE_314_API
>> void drvEtherIP_initHook ( initHookState state )
>> {
>>     if (drvEtherIP_private.lock == 0) return;
>>     if ( state == initHookAfterScanInit ) {
>>         epicsMutexLock(drvEtherIP_private.lock);
>>         databaseIsReady = true;
>>         epicsMutexUnlock(drvEtherIP_private.lock);
>>         drvEtherIP_restart();
>>     }
>> }
>> #endif
>> 
>> long drvEtherIP_drvInit ()
>> {
>>     /*
>>      * astonished to discover that initHookRegister is
>>      * in initHooks.h in R3.13, but not in iocCore
>>      * object file in R3.13
>>      */
>> #ifdef HAVE_314_API
>>     int status = initHookRegister ( drvEtherIP_initHook );
>>     if ( status ) {
>>         errlogPrintf (
>>               "drvEtherIP_drvInit: init hook install failed\n" );
>>     }
>> #endif
>>     return 0;
>> }
>> 
> 1394c1436
> <     long (* inie) ();
> ---
>>     long (* init) ();
> 1399c1441
> <     NULL
> ---
>>     drvEtherIP_drvInit
> 
> Jeff
> ______________________________________________________
> Jeffrey O. Hill           Email        johill@lanl.gov
> LANL MS H820              Voice        505 665 1831
> Los Alamos NM 87545 USA   FAX          505 665 5107
> 
> Message content: TSPA
> 
> With sufficient thrust, pigs fly just fine. However, this is
> not necessarily a good idea. It is hard to be sure where they
> are going to land, and it could be dangerous sitting under them
> as they fly overhead. -- RFC 1925
> 
> 
> 
>