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

Dispose issue in provider architecture part 2 #6

Closed
NeKoFu opened this issue Jun 5, 2019 · 12 comments
Closed

Dispose issue in provider architecture part 2 #6

NeKoFu opened this issue Jun 5, 2019 · 12 comments

Comments

@NeKoFu
Copy link

NeKoFu commented Jun 5, 2019

Hi,

In provider architecture pt2 there is an issue when views and widgets derived from BaseView are disposed.

To reproduce:
Launch the app, login to fetch and show posts on the homeview then go back to login screen.
You will see the FlutterError exception in the debug console content thrown by ChangeNotifier.

I/flutter (30244): ══╡ EXCEPTION CAUGHT BY WIDGETS LIBRARY ╞═══════════════════════════════════════════════════════════

I/flutter (30244): The following assertion was thrown while finalizing the widget tree:

I/flutter (30244): A HomeModel was used after being disposed.

I/flutter (30244): Once you have called dispose() on a HomeModel, it can no longer be used.

I/flutter (30244):

I/flutter (30244): When the exception was thrown, this was the stack:

I/flutter (30244): #0      ChangeNotifier._debugAssertNotDisposed.<anonymous closure> 

I/flutter (30244): #1      ChangeNotifier._debugAssertNotDisposed 

I/flutter (30244): #2      ChangeNotifier.dispose 

I/flutter (30244): #3      _BaseViewState.dispose 

I/flutter (30244): #4      StatefulElement.unmount 

I/flutter (30244): #5      _InactiveElements._unmount 

I/flutter (30244): #6      _InactiveElements._unmount.<anonymous closure> 

I/flutter (30244): #7      ComponentElement.visitChildren 

I/flutter (30244): #8      _InactiveElements._unmount 
...
@FilledStacks
Copy link
Owner

Hi, this should be fixed in the latest code. If you register the HomeModel as a Factory this should go away?. Unless it's the new Dispose I mentioned to add in the BaseView. I'll look into it when I get some time and will update here when I find a fix.

Thanks for letting me know.

@FilledStacks
Copy link
Owner

Sorry about the close

@FilledStacks FilledStacks reopened this Jun 5, 2019
@NeKoFu
Copy link
Author

NeKoFu commented Jun 5, 2019

In fact, I get the issue firstly on my own version then I have tried your own code yesterday and the issue appear in both codes.

But I will refetch your repository immediatly and try again.

@NeKoFu
Copy link
Author

NeKoFu commented Jun 5, 2019

I checked, HomeModel is registred as a Factory in the locator.dart and issue still here.

@FilledStacks
Copy link
Owner

I see. I'll check it out tonight while I'm busy writing the new tutorial and see what the problem is. Will update as soon as I find and fix it.

@NeKoFu
Copy link
Author

NeKoFu commented Jun 5, 2019

Ok. By the way, thank you for your greats tutorials on Flutter. It helps and inspires me a lot.

@FilledStacks
Copy link
Owner

Thanks man. It makes me happy to hear that.

@GIfatahTH
Copy link

ViewModels extend ChangeNotifier and the latter is disposed by flutter framework when the HomeView is popped out. Explicitly disposing HomeView and calling super.dispose will dispose the ChangeNotifier for the second time which it is not allowed.
@FilledStacks add dispose method to BaseView after a discussion with the creator of the provider package. But it is the StreamController in the AuthenticationService that needs to be disposed and AuthenticationService does not extend BaseModel.

@FilledStacks
Copy link
Owner

@GIfatahTH Thanks for looking into it, that makes sense. So I'll remove that call since it's already being called.

On the stream controller, it definitely should not be getting disposed during the entire lifetime of the app. It's used on the Home and posts view so disposing it at anytime would not be beneficial to the app.

The main problem was the additional dispose call in the base view, that was a silly mistake on my side. I should have realised it's already being called by provider and all we have to do is just override in the Model and dispose resources that we might want to dispose there.

Thanks for looking into it. Much appreciated.

@FilledStacks
Copy link
Owner

The reason for the LikeButtonModel exception is because the provider package already disposes the ChangeNotifier (our model) so all we have to do is override the dispose call inside the model and do our clean up in there.

@GIfatahTH
Copy link

Yes. for this particular case the stream controller should not be getting disposed during the entire lifetime of the app. But what about the general cas when I want to dispose resources from services.

@FilledStacks
Copy link
Owner

FilledStacks commented Jun 6, 2019

In this architecture if a stream is being used, outside of the provider. Through a model for instance. It means the subscription will live in the model. If you want to revoke that subscription to clean up the stream you'll override dispose in the model and do that.

There's no other scenario where you provide a stream to the entire app and you want to dispose that stream while the app is running. So any cleanup will be done per model if you have a subscription to a stream when every instance is created for that model.

class PostModel extends BaseModel {

MyService _service = locator<MyService>();

  StreamSubscription<Post> subscription;

  PostModel(){
    subscription = _service.onceOffStream.listen((data){
    // things
    })
  }

  @override 
  void dispose() {
    super.dispose()
    subscription.cancel();
  }
}

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

3 participants