ioc_container icon indicating copy to clipboard operation
ioc_container copied to clipboard

Does transient instances support disposing?

Open jinyus opened this issue 1 year ago • 9 comments

Correct me if I'm wrong... I don't think this is supported but this snippet in your docs implies that it is.

final builder = IocContainerBuilder()
    ..add((container) => DatabaseConnection('my-connection-string'))
    ..add<UserRepository>(
      (container) => UserRepository(container<DatabaseConnection>()),
      dispose: (userRepository) => userRepository.dispose(),
    )

Based on the dispose extension, only scoped singletons are disposed. Therefore the dispose function in the above example will never be called.

https://github.com/MelbourneDeveloper/ioc_container/blob/313e32d294a9a19b2edf77e7e61d001c574ffb51/lib/ioc_container.dart#L221-L234

jinyus avatar Jan 06 '24 12:01 jinyus

Hi @jinyus

Good point!

I will fix the code example. Thanks for pointing it out

A PR to fix the doco would be welcome

MelbourneDeveloper avatar Jan 06 '24 20:01 MelbourneDeveloper

I was looking through the code while creating the PR and realized that there is nothing wrong with the docs because the services are treated as singletons when scoped; so the dispose method will be called.

There is a conundrum though, because factory singletons cannot have a dispose method:

https://github.com/MelbourneDeveloper/ioc_container/blob/313e32d294a9a19b2edf77e7e61d001c574ffb51/lib/ioc_container.dart#L24-L27

So there is a memory leak where singletons created in a scope will never be disposed of. Let me know if you're ok with allowing singletons to have a dispose method and I will submit the PR.

jinyus avatar Jan 07 '24 01:01 jinyus

Sorry, yes, stored services do need dispose.

It's not a memory leak. That's by design. If you create a scope with disposable services, you need to dispose of them when you're finished.

MelbourneDeveloper avatar Jan 07 '24 01:01 MelbourneDeveloper

re: memory leak I mean that factory singletons are never disposed in a scope because you cannot supply a dispose method when registering them. My PR fixes this.

eg:

final builder = IocContainerBuilder()
    ..addSingletonService(AuthenticationService())

final container = builder.toContainer();

final scope = container.scoped();

final authService = scope<AuthenticationService>();
print(authService.login('user', 'password'));

await scope.dispose(); // AuthenticationService is not disposed here

jinyus avatar Jan 07 '24 01:01 jinyus

You can

You supply the dispose methods when you create the original container

Get Outlook for Androidhttps://aka.ms/AAb9ysg


From: jinyus @.> Sent: Sunday, January 7, 2024 12:11:04 PM To: MelbourneDeveloper/ioc_container @.> Cc: Christian Findlay @.>; Comment @.> Subject: Re: [MelbourneDeveloper/ioc_container] Does transient instances support disposing? (Issue #14)

re: memory leak I mean that factory singletons are never disposed in a scope because you cannot supply a dispose method when registering them. My PR fixes this.

— Reply to this email directly, view it on GitHubhttps://github.com/MelbourneDeveloper/ioc_container/issues/14#issuecomment-1879902754, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AD7MRS4SO3GM3PJFYKWRKRTYNHY2RAVCNFSM6AAAAABBPQRHDSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZZHEYDENZVGQ. You are receiving this because you commented.Message ID: @.***>

MelbourneDeveloper avatar Jan 07 '24 01:01 MelbourneDeveloper

Here is the code for addSingletonService:

https://github.com/MelbourneDeveloper/ioc_container/blob/313e32d294a9a19b2edf77e7e61d001c574ffb51/lib/ioc_container.dart#L153-L158

It doesn't allow you to supply a dispose method. So my code above would result in a memleak. Unless I'm mistaken.

jinyus avatar Jan 07 '24 01:01 jinyus

You can see here that the scoped method copies the whole service definition, which includes the dispose method

Screenshot_20240107-121824.png

MelbourneDeveloper avatar Jan 07 '24 01:01 MelbourneDeveloper

Yes, but currently there is no way to supply a dispose method for Singletons, only transients allow it.

eg:

  ///Add a singleton service to the container.
  void addSingletonService<T>(T service) => addServiceDefinition(
        ServiceDefinition<T>(
          (container) => service,
          isSingleton: true,
        ),
      );

  ///1️⃣ Add a singleton factory to the container. The container
  ///will only call this once throughout the lifespan of the container
  void addSingleton<T>(
    T Function(
      IocContainer container,
    ) factory,
  ) =>
      addServiceDefinition<T>(
        ServiceDefinition<T>(
          (container) => factory(container),
          isSingleton: true,
        ),
      );

addSingleton and addSingletonService doesn't allow you to supply a dispose method.

jinyus avatar Jan 07 '24 01:01 jinyus

@jinyus ah yes! I see your point. I've have a look at this and see if there are any issues

Thanks 🙏🏼

MelbourneDeveloper avatar Jan 07 '24 02:01 MelbourneDeveloper