Re: How to safely implement an asynPortDriver with SCAN records

[Mark Rivers via Tech-talk <tech-talk at aps.anl.gov>]

I agree that it is better to push the disable to asynManager, and not have to do it in each driver.  If necessary we could even add a new method to asynManager to permanently disable it, so Michael's worry about it being re-enabled would not be possible.

I believe that if the port is ASYN_MULTIDEVICE then pasynManager->enable(pasynUser, 0) must be called with pasynUser set to addr=-1 to disable the port itself, and not just a specific device.

Mark

---

From: Jure Varlec <jure.varlec at cosylab.com>
Sent: Thursday, July 21, 2022 7:15 AM
To: Michael Davidsaver <mdavidsaver at gmail.com>
Cc: Érico Nogueira Rolim <erico.rolim at lnls.br>; Mark Rivers <rivers at cars.uchicago.edu>; Andrew Johnson <anj at anl.gov>; tech-talk <tech-talk at aps.anl.gov>
Subject: Re: How to safely implement an asynPortDriver with SCAN records

 

Don't get me wrong, I don't disagree with you. Your suggested approach

definitely crosses all the ts and dots all the is. I just wanted to know whether

there's a problem with simply disabling the port that you are aware of and I'm

not. I'm sure you understand that I, and possibly Érico and others who have

existing drivers with existing cleanup code, have a judgment call to make. Your

suggested approach may be Best Practice in this context, but in general, so is

RAII, and they are not quite aligned. What you're proposing requires changing

the design of the driver such that it can be made defunct and resources released

without actually destroying it. And doing a redesign of a working driver has to

be weighed against simply disabling the port before destroying it, which seems

to work well in practice.

Another thing I'd like to point out is that your proposal moves the

responsibility of preventing the read and write calls to each driver. Which

works, of course. But I find it much more elegant to have these checks further

up the stack. Which, for the case of IOC scan threads which go through asyn

device support that, in turn, calls asynManager's queueRequest method, is

already the case! Don't you think it would make sense to enshrine (and perhaps

improve) this behavior and make life simpler for the drivers? I mean, doing

proper clean up in EPICS is fraught, this could be an opportunity to make things

a little bit better.

Best,

Jure

---

From: Michael Davidsaver <mdavidsaver at gmail.com>
Sent: Wednesday, July 20, 2022 23:43
To: Jure Varlec <jure.varlec at cosylab.com>
Cc: Érico Nogueira Rolim <erico.rolim at lnls.br>; Mark Rivers <rivers at cars.uchicago.edu>; Andrew Johnson <anj at anl.gov>; tech-talk <tech-talk at aps.anl.gov>
Subject: Re: How to safely implement an asynPortDriver with SCAN records

 

Caution: This email originated from outside of Cosylab.


On 7/20/22 11:51, Jure Varlec wrote:
>  > imo. it is reasonable to join worker threads here, but I would
>>recommend against deleting.
>>
>>Please keep in mind that delete leaves dangling pointers in the
>>asyn port.  imo. this is an invitation for the chaos of
>>use-after-free errors.
>
> Could be, but I can't see how.

Ah, some Famous Last Words :)


> The IOC is shutting down, asynManager is rejecting queueRequest calls to a disabled port

Which could be re-enabled.  And assuming that all code
actually respects the enabled switch.


> (which is what the device support code uses), and asyn doesn't register any hooks that I can see.

Currently.


> So AFAICT, when the records are the only users of the port, there should be no chaos.

As far as has been said, no.  This is an assumption through.

Actually, I should have mentioned before now the "atExitDebug"
global, which logs each exit hook before it runs.  I added this
after once being surprised by an exit hook I didn't know about.

> var atExitDebug 1


> Are you just trying to be careful,

In short, yes.

I'm trying to be careful and encourage what I see a good practice.
I certainly don't intend to jump on you in particular.

Tech-talk is an archived public mailing frequented by new(er)
users.  So in addition to Érico, I'm conscious of the audience.
Both now, and those whom google will later bring.


> or are you aware of something I missed? If I'm wrong about this, I'd love to know before I have to face the chaos :) .
>
> Best,
> Jure