-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ophyd v2 and PVAccess #836
Comments
Until ophyd advances, we have no official ophyd PVA interface. |
FYI @sveseli, the work would be done in the ophyd repo but once that settles down, there will be some work needed in apstools. This issue documents the need. |
I've done some testing about whether different epics clients can coexist in the same interpreter in this repo: https://github.com/coretl/can_epics_clients_coexist The If we don't use a consistent set then we might get seg faults, but I was unclear on when this is a problem, so I made some tests. Changing the import order gives unexpected results:
I can't make any sense of this, and I'm not sure how to proceed. @mdavidsaver, @sveseli, @anjohnson do you have any ideas? |
I am also not so clear. The circumstances are likely platform specific. eg. linux/ELF vs. windows/PE handle library loading and symbol lookup differently. (I assume you are testing on linux?)
You might have a look the process memory map to make sure only one copy of each library is in fact being loaded. On Linux this (very verbose) information is found in |
I suspect this would not be true on Windows, where there is no analog for the |
Yes, I am testing on Linux, although I have since added CI to test on Windows too. I couldn't get gdb working on Mac in CI after an hour of trying so I gave up on that.
The Linux seg faults:
The last 2 seg faults are understandable, there are 2 versions of
The windows results are more like what I expected to see:
Looks like loading 2 python modules with conflicting DLLs causes the non-epicscorelibs one to fail to load the DLL. It doesn't seg fault though |
Neither have I. However, OSX (at least in the github actions images) does some of this automatically as |
This is a crash on exit via |
I added it, and it works well for Linux, but doesn't appear to do anything for Windows or Macos, am I doing something wrong? |
I'll disable the atexit hook on both in turn and see which one fails... |
Ok, so either works on its own, so it looks like both are trying to destroy the same ca_context. In my actual application they both run in different threads, but the atexit function runs in the main thread so calls |
This can't be done as such. Instead, imo. a library/module using libca contexts should take steps to create its own context, and prevent other code from observing it. Also to prevent accidental usage of a context being managed by other code. The use of thread locals makes this awkward. Here is one example where I use a c++ class to manage a private libca context, and Attach it temporarily when calling ca_* functions. |
Ok, as my usecase is getting aioca and pyepics to coexist, and I can make aioca lazily create the context, where pyepics eagerly makes a context, and they both want a pre-emptive context, is there any reason not to make aioca use pyepics context if it already exists? As long as the lifetime of the context is the lifetime of the program (it is destroyed with an atexit hook) are there any downsides to sharing the context? |
Latest test results where aioca and pyepics don't destroy each other's contexts: @mdavidsaver I implemented the context sharing / destroying on the context we create in DiamondLightSource/aioca#36. I'm aware that switching in and out the contexts around each ca call would be more robust, but it's a lot less invasive to share the context between aioca and pyepics. I'd be interested to hear your thoughts on this... @prjemian How urgently do you need pvapy support? Will you be switching entirely to PVA as part of ophyd.v2 or adding it gradually? As the tests show, without a common base libCom the results are unpredictable. The Linux test loading @sveseli I think this has convinced me we need epics-base/pvaPy#80 in order to support pvapy in ophyd.v2 in a robust way |
I see that you have |
Current plans in ophyd v2 call for initial integration with p4p:
@coretl: This would be the point at which to choose pvapy instead.
Update: follow this PR
The text was updated successfully, but these errors were encountered: