Remove singletons when re-registering the same dependency

[michael-rubel]

Hi again @bobthemighty 👋

I have the following use case:

def client_input_closure():
    client_input: ClientInput = self.client_input_factory.create(data=body)

    self.dispatcher.dispatch(ClientInputCreated(data=client_input.model_dump()))

    return client_input

request.app.container.register(
    service=ClientInput,
    factory=client_input_closure,
    scope=Scope.singleton,
)

The attention is to scope=Scope.singleton.

If I want to have a scoped object creation (per request) without re-creating it for each service that requested the ClientInput object, I need to add the following hack:

if ClientInput in request.app.container._singletons:
    del request.app.container._singletons[ClientInput]

Making the object transient is safe, but performance will degrade depending on the application's size, as a new instance will be created for each class that requests this dependency.

I thought of overriding the register method in punq to delete the singleton if the application tries to re-register the same dependency with the singleton scope. Does this solution make sense to you or maybe this should be resolved differently?

[bobthemighty]

As before, I think your use case is best supported by a scoped container, or a request scoped lifecycle for dependencies.

I'll have a think about what that might look like. The fiddly bit is that it requires integration with the web framework to know when to clean up.

[michael-rubel]

Scoped container is probably not the best solution in our case, as it will force us to maintain two types of container in the application lifecycle. The first one is booted in the worker, completely out of the request scope. I think it's way easier to clean up singletons if the application requests re-register, something like this:

def register(
    self,
    service,
    factory=empty,
    instance=empty,
    scope=Scope.transient,
    **kwargs
):
    # Like the original, but with this if:
    if scope == Scope.singleton and service in self._singletons:
        del self._singletons[service]

    self.registrations.register(service, factory, instance, scope, **kwargs)

    return self

Do you see any pitfalls in this approach?

[bobthemighty]

I think it's way easier to clean up singletons if the application requests re-register,

I think this is a hack, and I dislike hacks because they make the codebase worse over time and lead to unexpected behaviours. You've declared a singleton, and you want to be able to say "oops, sorry, actually it's not a singleton, it has some other scope that we'll manage, don't worry about it."

I think that this raises questions about thread-safety, and idempotency/ordering of configuration code that I really don't want to consider. I think it's a footgun. I think that you can easily maintain a second container, for example with a decorator (chatgpt provided code, don't judge me).

@inject_container()
def my_function(container=None):
    greeter = container.resolve(Greeter)
    greeter.say_hello()

and then

# Assume a container available to this module
container = Container()

def inject_container(func):
    """
    A decorator to inject a punq container into a function as a kwarg.

    Parameters:
        func (Callable): The function to wrap.

    Returns:
        Callable: The wrapped function with the container injected.
    """
    @wraps(func)
    def wrapper(*args, **kwargs):

        # spawn new container, inheriting dependencies from the parent
        child_container = container.create_child()

        # add some additional thing
        child_container.register(Greeter, PrintGreeter)

        # invoke a wrapped function with the container
        kwargs['container'] = child_container
        result =  func(*args, **kwargs)

        # tear down any dependencies in the child container
        child_container.tear_down()

        # and return the result of our func
        return result

    return wrapper

We can figure out how to make this nicer to use, eg container.create_child(some_initialisation_func). The create_child function would likely set up a chained dict, so that it can explicitly override and replace some things in the parent container, as well as add new things, while inheriting the parent configuration otherwise.

This approach means that when you start a new web request, or a new thread, or whatever scope you want, you can create a new container that is just for you where you can mess about to your heart's content, and clean up when you're done.

Also, happy new year!

[michael-rubel]

@bobthemighty Hmm, I wonder if achieving this without the decorator is possible, maybe do it "behind the scenes". For example, to fork the container during application lifespan, but it will be the same object for classes that resolve the key after it's forked.

P.S. Happy New Year!

[bobthemighty]

Edit: just read your link, which happens to be to fastapi. I think that it a perfect extension point to create and destroy a child container. I'll leave the comment untouched.

Maybe! The question is "what does 'after' mean?"

If you have a global singleton container, and you're running, say, FastApi to create asynchronous endpoints, then requests can be interwoven. It's simply not true that one request executes, in its entirety, after another.

To handle that, you need to store some scoped information. For example, we could register the child container as a dependency, so it can be resolved, and use ContextVars to make sure the correct request has access to the correct container.

If the application is using threads, then we'd need ThreadLocalStorage.

Knowing how to scope a particular request is the responsibility of the consumer, because it can't be done generically by the library, and it's unsafe to assume that you can change global state in a singleton repository, and so I wouldn't build that into the API.

There are almost certainly ways that we can make this neater, but you have to start from "how do I scope the lifetime of this dependency so that the correct instance is available to the correct request", and the solution has to be applicable in a range of use cases.

[michael-rubel]

Tried the "scoped" container by copying the original. It seems that both solutions are not quite safe/useful. We have multiple use cases when dependencies in class constructors are mixed between both scopes (i.e. the class requires data from request, but also state from the object previously registered in the main container and then shared memory executor involved). If we maintain two different bags, this mixing won't be possible because objects inside them will have totally different references, thus state and resolve call stack. There are cases when the state should be kept between requests, when not, and both should be accessed.

I'd like to avoid developer care about memory management of core components, i.e. he shouldn't consider types of similar objects he needs to use when adding a new class to the application.

I think it would make sense to add a third "request" scope (request-scoped singleton) and then let the user decide where to clean up the state (depending on the framework), but that would be a huge new feature for the punq that will also require thread-safety measures.

[bobthemighty]

@michael-rubel can you give me some example use-cases where you think the scoped container won't work? I'm not planning to create a copy, I'm planning to create a chained dict.

In that model, we have some dictionary

original = {
  "a": 1,
  "b": 2
}

and we want to create some new clone

cloned = clone(original)
cloned["c"] = 3

The clone isn't a copy, it's shadowing the original, so that.

assert cloned["a"] == 1
assert cloned["c"] == 3

original["a"] == 2
cloned["b"] == "wut"

assert cloned["a"] == 2
assert cloned["b"] == "wut"

Does that cover your needs? Ie, the clone isn't a new reference, it's a pointer to the original reference, that can be shadowed in scope.

[bobthemighty]

@michael-rubel does this solve your problem?

https://github.com/bobthemighty/punq/pull/197/files#diff-b49c2f54db881cf638f7fca2d3cb39bb9967deb2ba26fa9dd137a5c9176442a6R119

[michael-rubel]

@bobthemighty It looks like it works in general, but my factory executes two times on the initial request.

Code:

def set_client_input(self, body: dict):
    def closure():
        client_input = self.client_input_factory.create(data=body)

        self.dispatcher.dispatch(ClientInputCreated(data=body))

        return client_input

    self.container.register(
        service=ClientInput,
        factory=closure,
        scope=Scope.singleton,
    )

Event ClientInputCreated is dispatched two times in the initial request, then once on every subsequent request. It seems related to the fact that some of the dependencies are resolved via the original container, and some from the child, then both save this singleton thus creating one instantiation on the shared container, and one on the child per request:

@routes.post("/main")
async def main(request: Request):
    container: Container = request.app.container.child()

Can we pass a singleton into the child instead of instantiating the object separately, but only if it exists in the parent?

[bobthemighty]

Can we pass a singleton into the child instead of instantiating the object separately, but only if it exists in the parent?

I don't think this is what you want, though - the container will already walk up the chain to find a dependency if it's not registered locally. If it is registered locally, we'll return the local one.

The question is "Why do you need to register ClientInput in the parent scope?" It seems to me that in your FastAPI layer, you want to do

app.container = root_container.child()
app.container.register(ClientInput, closure, scope=Scope.singleton)

and not to register it in parent scope at all.

If you do want a ClientInput in the parent for some reason (eg. an EmptyClientInput) then by definition that will need to be created separately.

[michael-rubel]

We have the context where we use a shared container because a lot of dependencies are registered during the server boot time (like when you start "uvicorn" and spawn the worker). Then there's a stateful executor that works independently from the request (the server keeps it in memory after it started), but is dependent on the incoming input from every request (i.e. we need to feed unique data from the request to that stateful executor).

[bobthemighty]

I'm deeply confused - what ClientInput do you register at boot time?

I'm merging the PR as is, but I'm happy to accept more feedback.

[michael-rubel]

I don't register it during boot time, only during request, but the object is requested by classes in the stateful executor (alongside dependencies registered during boot time).

[bobthemighty]

Okay!

So you have two code execution paths in your system:

1. A FastAPI http service and
2. Some kind of background processor, which lives in the same process.

When you handle a web request, you want to pass work to the stateful processor, but that requires access to the client input, which you're injecting through DI - do I have that right? How do you invoke the stateful worker?

OR

The stateful processor does not require the ClientInput, but it uses some classes that have a dependency on it?