Code review comment for lp:~matej-sekoranja/epics-base/ca-username-change

Revision history for this message
Matej Sekoranja (matej-sekoranja) wrote :

> > Based on my experience with bug lp:1091401 testing need to include potential
> > races with re-loading the ACF file with asInitAsyn().
>
> I've convinced myself that there is at least one potential deadlock here.
>
> asInitAsyn() parses the new ACF file then triggers the asCaTask to call
> asComputeAllAsg() which locks the global asLock then (eventually) calls
> asComputePvt(). Calls to asComputePvt() may trigger some callbacks to
> casAccessRightsCB(), which locks chanListLock.
>
> Your modified client_name_action locks chanListLock then calls
> asChangeClient(), which locks asLock and calls asComputePvt() as well.
>
> So to order of locking between asLock and chanListLock is reversed in these
> two potentially concurrent paths.
>
> So a deadlock is possible in the rather unfortunate situation that a CA client
> does a put which triggers asInitAsyn(), then quickly changes its client name
> at the same time as its write permission is to this record revoked (due to ACF
> policy changes).

I would suggest to lock asLock before locking chanListLock. This establishes proper lock ordering.
The assumption is that the lock is recursive. Are locks in EPICS base recursive?

« Back to merge proposal