Experimental Physics and
Industrial Control System

Michael Davidsaver via Core-talk <[email protected]> · Fri, 25 Jan 2019 10:21:14 -0800

> I wonder using if the pointer to the record that received the original put would be sufficiently unique as an ID?

Remember that the goal here is fundamentally about detecting a recursive function
call path.  What needs to be tracked to remove ambiguity must be tied to the thread,
not any record(s) involved.  This is closer to PACT than to either PUTF or RPRO.

The question which needs to be answer in the db link code is: Is this thread already
already processing this (target) record further down in the stack?

Also, could you, as the dbNotify expert, look at my proposed changes?

https://code.launchpad.net/~epics-core/epics-base/+git/database/+merge/362207

On 1/24/19 3:49 PM, Johnson, Andrew N. wrote:
> On 1/24/19 12:54 PM, Michael Davidsaver wrote:
>> Seems like the best course is for dbProcess() and processTarget()
>> to keep track of which records are being recursed into.
> I hope you aren't considering generating lists or trees of records at runtime...

I'm not in this case, though I'm not sure I understand your concern?

I suppose what I am thinking of is conceptually similar to having each
thread involved in scanning use a thread local to keep a list of
record being processed.  However, this provides more information than
I think is needed in this case.

> BTW contrary to what I wrote below RPRO wouldn't need to store the ID, only PUTF would, although unfortunately PUTF is currently visible externally (but is SPC_NOMOD).
>>> I could see of a way around this by storing an identifier in the PUTF and RPRO fields which would let us see that (I'd make the PUTF and RPRO fields into DBF_LONG and use an atomic counter to generate a unique ID for each source put).
>> I was thinking of using a stack address, or maybe epicsThread*, from the thread which is doing
>> the processing.  Not really fundamentally different from a global counter, though maybe easier
>> to debug.  This would be set and cleared by dbProcess().
> Just noticed that epicsAtomicIncrIntT() implements undefined behaviour if it wraps because it operates on a signed integer, so we couldn't use that to generate a 'unique-enough' ID anyway.
> 
> BTW just changing the 'else if' in processTarget() to this
>>     else if (psrc->putf && psrc->putf != pdst->putf) {
> fixes Mark's database, but breaks asyncproctest:
> 
>> # ===== Chain 2 again ====== ok 11 - dbPutField("chain2:1.B", 5, ...) -> 0 (Success) ok 12 - dbPutField("chain2:1.B", 5, ...) -> 0 (Success) ok 13 - dbPutField("chain2:1.B", 5, ...) -> 0 (Success) Bail out! Processing timed out Abort 
> 
> I wonder using if the pointer to the record that received the original put would be sufficiently unique as an ID?

I think only if a loop doesn't involve any branching,
and always includes this head record.  I don't think either
can be assumed.

> When two puts come into the same record before async completion the second put would not set RPRO, but since RPRO is already true we'd just avoid setting it the second time. Using a dbCommon* also allows for even nicer debug/trace messages, we can print the put-record's name in the trace messages in processTarget().
>> I'm not sure about hijacking the existing flag fields.  Maybe add a new field in dbCommonPvt?
> The existing PUTF field is /so close/ to being sufficient; I would much prefer to use it for this than duplicate it, the external visibility is the main problem I see. Any driver/record (such as the tpmac) which writes or writes directly to the field will break, but nobody should be fiddling with dbCommon fields like that.
> 
> I'm working on adding a new special() type to allow read-only access to some dbCommon fields that need a little pre-processing — I've wanted to support getting the TIME field for years, so this is just another field to handle in the same way. My dbCommon *PUTF is now working with that...
> 
> I don't like hiding per-record data in pvCommonPvt. IMHO the recnode field there should have been another DBF_NOACCESS SPC_NOMOD extra() field added to dbCommon.dbd just like every other IOC subsystem field.

For a first pass I'll probably put any new fields to dbCommonPvt.
We can discuss moving to dbCommon after a solution is found.

This may allow a fix which maintains ABI compatibility with 7.0.2,
which was one of my reasons for introducing dbCommonPvt.

> - Andrew
> 
> -- 
> Arguing for surveillance because you have nothing to hide is no
> different than making the claim, "I don't care about freedom of
> speech because I have nothing to say." -- Edward Snowdon
> 

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System