Skip to content
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

Multiple DynamicProviders not handled correctly #133

Open
Synthetica9 opened this issue Jan 15, 2024 · 1 comment · May be fixed by #138
Open

Multiple DynamicProviders not handled correctly #133

Synthetica9 opened this issue Jan 15, 2024 · 1 comment · May be fixed by #138
Labels

Comments

@Synthetica9
Copy link

Description

Only one DynamicProvider is really supported, the second one is dispatched incorrectly.

To reproduce

Run the following server:

from p4p.server import Server, DynamicProvider
from p4p.server.thread import SharedPV
from p4p.nt import NTScalar

class DynHandler(object):
    def __init__(self, name):
        self.name = name
        self.pv = SharedPV(nt=NTScalar('d'), initial=0.0)

    def testChannel(self, name): # return True, False, or DynamicProvider.NotYet
        return name == self.name

    def makeChannel(self, name, peer):
        assert name == self.name
        return self.pv

providers = [
    DynamicProvider("foo", DynHandler("foo")),
    DynamicProvider("bar", DynHandler("bar")),
]
server = Server.forever(providers)

Then run pvget foo

Observed behaviour

bar is also called, raising an assertion error:

Unexpected
Traceback (most recent call last):
  File "/ics/tools/p4p/python3.8/linux-x86_64/p4p/server/__init__.py", line 238, in makeChannel
    return self._real.makeChannel(name, peer)
  File "/tmp/multi_p4p.py", line 14, in makeChannel
    assert name == self.name
AssertionError
TypeError: makeChannel("foo") must return SharedPV, not NoneType

Expected behaviour

Only foo channel is called.

Stopgap solution

class MergeHandler:
    def __init__(self, handlers):
        self.handlers = handlers

    def get_responsible(self, name):
        responsible = [
            handler for handler in self.handlers if handler.testChannel(name)
        ]
        if len(responsible) == 1:
            return responsible[0]

        elif len(responsible) == 0:
            return

        else:
            raise ValueError("More than one responsible handler~!")

    def testChannel(self, name):
        return self.get_responsible(name) is not None

    def makeChannel(self, name, peer):
        responsible = self.get_responsible(name)
        return responsible.makeChannel(name, peer)
@mdavidsaver
Copy link
Member

There are two related problems on display. A difference in behavior between pvAccessCPP and PVXS libraries which I didn't account for when updating P4P. Also, a different in how pvAccessCPP worked vs. how it was documented.

Even with pvAccessCPP there are situation where makeChannel() could be called with a PV name without a previous call to testChannel() with that name. eg. this is how the "magic" _server channel name is implemented. Still, was mostly hidden by how pvAccessCPP handled multiple Providers.

PVXS makes this explicit by requiring the implementations of Source.onCreate() handle direct connection without a previous search.

However, P4P does not correctly account for this. makeChannel() needs to be allowed to return None to indicate disinterest. (PVXS will then try the next provider)

https://github.com/mdavidsaver/p4p/blob/f5c96bad937fff57f5243f72b7fdb89a408d0f4a/src/pvxs_source.cpp#L117-L122

@mdavidsaver mdavidsaver linked a pull request Jan 19, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants