EPICS Controls Argonne National Laboratory

Experimental Physics and
Industrial Control System

1994  1995  1996  1997  1998  1999  2000  2001  2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  2017  2018  2019  2020  2021  <20222023  2024  Index 1994  1995  1996  1997  1998  1999  2000  2001  2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  2017  2018  2019  2020  2021  <20222023  2024 
<== Date ==> <== Thread ==>

Subject: Re: How to safely implement an asynPortDriver with SCAN records
From: Mark Rivers via Tech-talk <tech-talk at aps.anl.gov>
To: Jure Varlec <jure.varlec at cosylab.com>, Michael Davidsaver <mdavidsaver at gmail.com>
Cc: Érico Nogueira Rolim <erico.rolim at lnls.br>, tech-talk <tech-talk at aps.anl.gov>
Date: Thu, 21 Jul 2022 12:57:08 +0000
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


References:
How to safely implement an asynPortDriver with SCAN records Érico Nogueira Rolim via Tech-talk
RE: How to safely implement an asynPortDriver with SCAN records Mark Rivers via Tech-talk
Re: How to safely implement an asynPortDriver with SCAN records Érico Nogueira Rolim via Tech-talk
Re: How to safely implement an asynPortDriver with SCAN records Andrew Johnson via Tech-talk
RE: How to safely implement an asynPortDriver with SCAN records Mark Rivers via Tech-talk
Re: How to safely implement an asynPortDriver with SCAN records Michael Davidsaver via Tech-talk
Re: How to safely implement an asynPortDriver with SCAN records Érico Nogueira Rolim via Tech-talk
Re: How to safely implement an asynPortDriver with SCAN records Jure Varlec via Tech-talk
Re: How to safely implement an asynPortDriver with SCAN records Michael Davidsaver via Tech-talk
Re: How to safely implement an asynPortDriver with SCAN records Jure Varlec via Tech-talk
Re: How to safely implement an asynPortDriver with SCAN records Michael Davidsaver via Tech-talk
Re: How to safely implement an asynPortDriver with SCAN records Jure Varlec via Tech-talk

Navigate by Date:
Prev: Phoebus Scan Server Ian Gillingham via Tech-talk
Next: CS-Studio: File Manager configuration when executing external command Zhang, Tong via Tech-talk
Index: 1994  1995  1996  1997  1998  1999  2000  2001  2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  2017  2018  2019  2020  2021  <20222023  2024 
Navigate by Thread:
Prev: Re: How to safely implement an asynPortDriver with SCAN records Jure Varlec via Tech-talk
Next: Re: How to safely implement an asynPortDriver with SCAN records Michael Davidsaver via Tech-talk
Index: 1994  1995  1996  1997  1998  1999  2000  2001  2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  2017  2018  2019  2020  2021  <20222023  2024 
ANJ, 14 Sep 2022 Valid HTML 4.01! · Home · News · About · Base · Modules · Extensions · Distributions · Download ·
· Search · EPICS V4 · IRMIS · Talk · Bugs · Documents · Links · Licensing ·