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

Autoproxy lifetimes are broken #92

Open
TheSven73 opened this issue Jul 16, 2024 · 5 comments
Open

Autoproxy lifetimes are broken #92

TheSven73 opened this issue Jul 16, 2024 · 5 comments

Comments

@TheSven73
Copy link

TheSven73 commented Jul 16, 2024

I'm trying to take an existing Python library, and allow it to be used remotely over the network. The library creates and returns Python objects which contain C library handles, so AFAIK they cannot be serialized, and need to be auto-proxied.

So far so good. But, I am running into trouble with autoproxy object lifetimes. When an autoproxy proxy handle goes out of scope, the corresponding server object is never released.

Some pytests which I believe should succeed, yet two fail. Edit: python==3.12.3, Pyro5==5.15, pytest==8.2.2

from concurrent.futures import ThreadPoolExecutor

import pytest
from Pyro5.api import Daemon, Proxy, behavior, expose


@expose
class Child:
    def __init__(self, name: str, alive: set[str]):
        self.__name = name
        self.__alive = alive
        alive.add(name)

    def __del__(self):
        self.__alive.remove(self.__name)

    def __repr__(self):
        return self.__name

    def ping(self):
        return f"I am {self.__name}"


_live_objects: set[str]


@expose
@behavior(instance_mode="session", instance_creator=lambda clazz: clazz(_live_objects))
class Parent:
    _pyroDaemon: Daemon

    def __init__(self, alive: set[str]):
        self.__name = f"outer {id(self)}"
        self.__alive = alive
        alive.add(self.__name)

    def __del__(self):
        self.__alive.remove(self.__name)

    def __repr__(self):
        return self.__name

    def ping(self):
        return f"I am {self.__name}"

    def createInstance(self, weak=False):
        i = Child(name="single instance", alive=self.__alive)
        self._pyroDaemon.register(i, weak)
        return i

    def createGenerator(self, weak=False):
        ii = [
            Child(name=f"iterator instance {i}", alive=self.__alive) for i in range(4)
        ]
        for i in ii:
            self._pyroDaemon.register(i, weak)
            yield i


@pytest.fixture
def proxy():
    global _live_objects
    _live_objects = set()
    with ThreadPoolExecutor(max_workers=1) as e, Daemon() as daemon:
        uri = daemon.register(Parent, force=True)
        must_shutdown = False
        _ = e.submit(daemon.requestLoop, lambda: not must_shutdown)
        with Proxy(uri) as proxy1, Proxy(uri) as proxy2:
            yield proxy1, proxy2
        must_shutdown = True
    assert not _live_objects, "some objects were not cleaned up"


# succeeds
def test_proxy_lifetime(proxy):
    p1, p2 = proxy
    assert "outer" in p1.ping()
    assert "outer" in p2.ping()


# fails teardown: AssertionError: some objects were not cleaned up
def test_instance_lifetime(proxy):
    p, _ = proxy
    assert "outer" in p.ping()
    inner = p.createInstance()
    assert "single instance" in inner.ping()


# fails teardown: AssertionError: some objects were not cleaned up
def test_generator_lifetime(proxy):
    p, _ = proxy
    for inner in p.createGenerator():
        assert "iterator instance" in inner.ping()
@irmen
Copy link
Owner

irmen commented Jul 16, 2024

I think the daemon's shutdown() is never called and it remains running in the requestLoop().
Setting the must_shutdown to true after the call, is not magically going to change the value of the boolean argument to requestLoop(). Talking about the proxy() function above.

@TheSven73
Copy link
Author

TheSven73 commented Jul 16, 2024

The must_shutdown variable is captured in the lambda, which is a closure. Setting must_shutdown to True makes the lambda return False so that the requestLoop eventually exits. The ThreadPoolExecutor context manager blocks until all futures scheduled on the executor have finished. proxy() can only exit if the RequestLoop has stopped.

I clarified the code by waiting for the requestloop future to finish explicitly, however this produces the exact same result:

@pytest.fixture
def proxy():
    global _live_objects
    _live_objects = set()
    with ThreadPoolExecutor(max_workers=1) as e, Daemon() as daemon:
        uri = daemon.register(Parent, force=True)
        must_shutdown = False
        fut = e.submit(daemon.requestLoop, lambda: not must_shutdown)
        with Proxy(uri) as proxy1, Proxy(uri) as proxy2:
            yield proxy1, proxy2
        must_shutdown = True
        fut.result() # <==== added
    assert not _live_objects, "some objects were not cleaned up"

The fixture approach using an executor is perhaps too complex. I can create a test example where the daemon requestloop runs inside a thread, would you prefer this approach?

@TheSven73
Copy link
Author

Followed your suggestion to use daemon.shutdown() this works faster, great ! Same result though, the two tests still fail:

@pytest.fixture
def proxy():
    global _live_objects
    _live_objects = set()
    with ThreadPoolExecutor(max_workers=1) as e, Daemon() as daemon:
        uri = daemon.register(Parent, force=True)
        fut = e.submit(daemon.requestLoop)
        with Proxy(uri) as proxy1, Proxy(uri) as proxy2:
            yield proxy1, proxy2
        daemon.shutdown()
    assert not _live_objects, "some objects were not cleaned up"

@irmen
Copy link
Owner

irmen commented Jul 16, 2024

Hmm... okay I was mistaken about the boolean

It's been a few years since I wrote the unit-tests for pyro5, but there are a bunch in the test_server module that use a running daemon. Maybe I did something different there?
I am not sure if I ever cared about leaking refs in those tests though.

@TheSven73
Copy link
Author

TheSven73 commented Jul 17, 2024

I added a few simple (currently failing) tests that verify autoproxy lifetimes here: #93

Maybe these might be useful as a starting point to investigate the issue further?

@TheSven73 TheSven73 changed the title Are autoproxy lifetimes broken? Autoproxy lifetimes are broken Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants