jonataslaw / getx

Open screens/snackbars/dialogs/bottomSheets without context, manage states and inject dependencies easily with Get.
MIT License
10.43k stars 1.63k forks source link

getxController isn't being disposed/deleted when it should be #961

Open xEyad opened 3 years ago

xEyad commented 3 years ago

Describe the bug when i go from screen 1 to screen 2 i expect the controller to be deleted/disposed but this doesn't happen. and you can see this as the timer completes from where it stopped. i based my expectations on this document https://github.com/jonataslaw/getx/blob/master/documentation/en_US/state_management.md#simple-state-manager section How it handles controllers

Reproduction code

import 'dart:async';
import 'package:get/get.dart';
import 'package:flutter/material.dart';

runApp(sandbox());

dynamic sandbox()
{
  return GetMaterialApp(
        title: 'Getx issue',
        debugShowCheckedModeBanner: false,
        home: Prime()); 
}

class PrimeController extends GetxController {
  Timer _timer;
  Rx<Duration> recordedTime =  Duration.zero.obs;
  @override
  void onInit() {
    super.onInit();
    _timer = Timer.periodic(
        Duration(
          seconds: 1,
        ),
        (newVal) => recordedTime.value = recordedTime.value + Duration(seconds: 1)); 
  }

  @override
  void onClose() {
    _timer.cancel();
    //THIS IS NEVER HIT
    super.onClose();
  }
}

class Prime extends GetView<PrimeController> 
{
  Prime()
  {
    Get.put(PrimeController());
  }
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(child: 
      Column(
        mainAxisAlignment: MainAxisAlignment.center,
        children: [
        Obx(()=>Text('${controller.recordedTime.value}')),
        _nextPage(),
      ],)
    ,),
    );
  }
  Widget _nextPage()
  {
    return RaisedButton(onPressed: ()=>Get.off(SomePage()),child: Text('Next page'),);
  }
}

class SomePage extends StatelessWidget {
  const SomePage({Key key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: Text("SomePage"),),
      body: Center(
      child: RaisedButton(child: Text("Previous page"),onPressed: ()=>Get.off(Prime())),
    ),
    );
  }
}

To Reproduce Steps to reproduce the behavior:

  1. press 'Next page' button
  2. press "previous page" button
  3. notice the error (the timers completes from where it stopped)

Expected behavior the timer should reset as the controller was disposed and rebuilt

Screenshots none

Flutter Version: 1.22.5 stable channel

Getx Version: get: ^3.22.2

Describe on which device you found the bug: pixel api 28 5 inch - Android.

Minimal reproduce code same as above

jonataslaw commented 3 years ago

change:

Prime()
  {
    Get.put(PrimeController());
  }
  @override
  Widget build(BuildContext context) {

to:

  @override
  Widget build(BuildContext context) {
  Get.put(PrimeController());

The class constructor is called before the route, so it is attached to the previous route.

xEyad commented 3 years ago

@jasonlaw so, instead of adding the put inside the constructor i should add it to the build? if yes, i already did that and i got the same result..

eduardoflorence commented 3 years ago

@xEyad, after making the modification that @jasonlaw indicated, how was your Prime class? Put a print on the onClose method to make sure it doesn't go through it.

nastaran-mohammadi commented 3 years ago

same here! i go to a page like product counter page, firstly i enter to this page counter is 0 and GetXController print some thing that is on onInit() function, i set counter to 5 and then i press back button and close the page when i go to page for second time the counter value is 5 and onInit() function is not called again! also i write onClose() function too but it is not working. help me please! @xEyad @jonataslaw @jasonlaw @lsm

jonataslaw commented 3 years ago

Apparently the initial route is being appended to '/', however you are not using named routes. I'm doing a hotfix now for this

ali80 commented 3 years ago

same here! i go to a page like product counter page, firstly i enter to this page counter is 0 and GetXController print some thing that is on onInit() function, i set counter to 5 and then i press back button and close the page when i go to page for second time the counter value is 5 and onInit() function is not called again! also i write onClose() function too but it is not working. help me please! @xEyad @jonataslaw @jasonlaw @lsm

Had similar problem, for now you can use binding, everything works fine with that.

xEyad commented 3 years ago

same here! i go to a page like product counter page, firstly i enter to this page counter is 0 and GetXController print some thing that is on onInit() function, i set counter to 5 and then i press back button and close the page when i go to page for second time the counter value is 5 and onInit() function is not called again! also i write onClose() function too but it is not working. help me please! @xEyad @jonataslaw @jasonlaw @lsm

Had similar problem, for now you can use binding, everything works fine with that.

binding sounds like a lot of work that's why i didn't use it in the first place, and this also means either A. i will do this only for the current issue i have in the given screen only making an inconsistency with my whole codebase or B. change my whole codebase to use bindings instead of the current method.

but regardless of my preference i may have to resort to B because i won't ever know when i will encounter this issue again and it may go under the radar. i really do hope that jonataslaw fix this issue fast. so far GETX is the best state management library i have used and i really like it. anyways, if this problem won't be solved soon, may you tell me so that i adopt the alternative solution early instead of waiting for a fix.

sorry for the long rant.

ali80 commented 3 years ago

@xEyad take a look at BindingsBuilder, very easy to use

Get.to(MyPage(), binding: BindingsBuilder(() {
                        Get.put<GController>(GController());
                      }),
                    );
bonaparta13 commented 3 years ago

is the bug fixed ?

nastaran-mohammadi commented 3 years ago

I fix this bug with defining controller after build context, thanks all

On Sun, Jan 3, 2021, 3:18 PM F-BONAPARTA notifications@github.com wrote:

is the bug fixed ?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jonataslaw/getx/issues/961#issuecomment-753605241, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKXWV4A2GHQT574LFULW4C3SYBKQHANCNFSM4VNULOVA .

xEyad commented 3 years ago

is the bug fixed ?

nope

I fix this bug with defining controller after build context, thanks all interesting, that didn't fix it when i used this solution.

nastaran-mohammadi commented 3 years ago

is the bug fixed ?

nope

I fix this bug with defining controller after build context, thanks all interesting, that didn't fix it when i used this solution.

could you please share your code?

oliverbytes commented 3 years ago

I have the exact same issue with Timers.. even if GetX logs says the screen is fully deleted in memory, still the timer is still alive and kicking

loic-hamdi commented 3 years ago

same here! i go to a page like product counter page, firstly i enter to this page counter is 0 and GetXController print some thing that is on onInit() function, i set counter to 5 and then i press back button and close the page when i go to page for second time the counter value is 5 and onInit() function is not called again! also i write onClose() function too but it is not working. help me please! @xEyad @jonataslaw @jasonlaw @lsm

@jonataslaw @eduardoflorence

With get: ^3.26.0 I experience a similar behavior but when I use id in Get.to()

FlatButton(
                          onPressed: () {
                            Get.to(() => DummyView(), binding: BindingsBuilder(() {
                              Get.put<DummyController>(DummyController());
                            }));
                          },
                          child: const Text(
                            'Dummy view',
                          ),
                        ),
class DummyController extends GetxController {
  final RxInt counter = 0.obs;

  void increment() {
    counter.value++;
  }

  @override
  void onInit() {
    logger.w('Dummy controller launched, counter value : $counter');
    super.onInit();
  }
}
class DummyView extends GetView<DummyController> {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: SafeArea(
        child: Center(
          child: Column(
            children: [
              Obx(
                () => Text('Counter value : ${controller.counter.value.toString()}'),
              ),
              RaisedButton(
                onPressed: () {
                  controller.increment();
                },
                child: const Text('Increment counter'),
              )
            ],
          ),
        ),
      ),
    );
  }
}

The controller is not deleted from memory if I use id

Get.to(() => DummyView(), id: 1, binding: BindingsBuilder(() {
                              Get.put<DummyController>(DummyController());
                            }));

Same problem with named routes

eduardoflorence commented 3 years ago

@zzterrozz, Why are you using id in Get.to? We only use id when we are working with Nested Navigation. https://github.com/jonataslaw/getx/blob/master/documentation/en_US/route_management.md#nested-navigation

jimbengtsson92 commented 3 years ago

We have the same use case as @zzterrozz. We use nested navigation in our app, and it seems like no Controllers are ever deleted.

alionour commented 3 years ago

any updates on this bug

jimbengtsson92 commented 3 years ago

From our experience a controller is never disposed when using Get.put(controller) or Get.lazyPut(() => controller). If the widget creates the controller by using a GetX<Controller> or GetBuilder<Controller> then the controller is disposed correctly. For example

GetX<Controller>(
      init: Controller(),
      builder: (controller) => Scaffold(...)
)

disposes the controller correctly in our cases.

OliverRhyme commented 3 years ago

Same here. Any updates??

stjfk commented 3 years ago

From our experience a controller is never disposed when using Get.put(controller) or Get.lazyPut(() => controller). If the widget creates the controller by using a GetX<Controller> or GetBuilder<Controller> then the controller is disposed correctly. For example

GetX<Controller>(
      init: Controller(),
      builder: (controller) => Scaffold(...)
)

disposes the controller correctly in our cases.

The problem I mentioned earlier may also be related to this https://github.com/jonataslaw/getx/issues/1299

aboda1986 commented 3 years ago

Any update about this issue?

pontino commented 3 years ago

Hello, im using Nested navigators, and i can't have a controller automatically disposed when navigating by id, even with a binding associated with it.

Im forced to use GetX<Controller>.

From your documentation:

GetX is still more economical than any other reactive state manager, but it consumes a little more RAM than GetBuilder. Thinking about it and aiming to maximize the consumption of resources that Obx was created

That's why this is an issue, we cannot use efficients solution using nested navigation

henriqueArrazao commented 3 years ago

I'm experiencing a similar issue, if I "Get.offAll" from the first screen, the controller never disposes, any news about this issue?

Maqsood0 commented 2 years ago

Same issue, Getx controller is deleted every time when running app on emulator but after generating Apk , it does not delete controller from memory, hence causing the page to stuck to it's first view when the app begins. Any suggestion or solution regarding this?

tahamv commented 2 years ago

same issue , getx controller doesnt disposed when i changed page.

tahamv commented 2 years ago

I find solution for my problem I used a progress indicator and the addAutomaticKeepAlive was true which prevent getxcontroller from being disposed if you face with this problem just search for stream or animation that doesnt disposed and prevent controller from being disposed. @henriqueArrazao @Maqsood0 @aboda1986

Saeeed-B commented 2 years ago

This is my first time using a filter, this is really annoying. @jonataslaw Please provide a solution

gabrielgt3k commented 2 years ago

same issue here using nested navigation, the controller from generated nested routes are not being disposed. Also, the Getx nested navigation docs is poor...

davidpanic commented 1 year ago

This is still an issue, is there any fix available other than using GetBuilder or GetX? Bindings are really annoying to deal with, I wouldn't even mind if I had to extend some other class instead of StatefulWidget but without using bindings.

saranyalekshmanan commented 1 year ago

Please check this link https://stackoverflow.com/questions/73225356/what-is-the-best-way-to-dispose-resources-in-getx-flutter. I also used a timer and I have cancelled it on the Onclose function

saranyalekshmanan commented 1 year ago

I have the exact same issue with Timers.. even if GetX logs says the screen is fully deleted in memory, still the timer is still alive and kicking

Please check this link . https://stackoverflow.com/questions/73225356/what-is-the-best-way-to-dispose-resources-in-getx-flutter

praveen12q commented 1 year ago

define binding class in diffrent files.not use common file for define binding

FarhanSyedain commented 1 year ago

If you are using statefull widget , here is a quick get around

void dispose() { Get.delete<MyController> super.dispose}

minegulov98 commented 1 year ago

Noticed, that on pressing physical back button (or back button in appbar) onClose is not called until i do some actions on previous screen. But when using Get.back() with button on screen, controller disposes correctly. With leading button in AppBar this doesn't work though. I'm using bindings with named routes. How to make same behaviour when using back button? It also says, that route is closed: CLOSE TO ROUTE in console But controller is not being disposed

KentaIbayashi commented 1 year ago

Try update Getx to 4.6.6. It solved like a magic in my case.

AClon314 commented 1 year ago

If you are using statefull widget , here is a quick get around如果您使用的是有状态小部件,这里有一个快速的解决方法

void dispose() { Get.delete<MyController> super.dispose}

Useful for me ❤

minegulov98 commented 1 year ago

Noticed, that on pressing physical back button (or back button in appbar) onClose is not called until i do some actions on previous screen. But when using Get.back() with button on screen, controller disposes correctly. With leading button in AppBar this doesn't work though. I'm using bindings with named routes. How to make same behaviour when using back button? It also says, that route is closed: CLOSE TO ROUTE in console But controller is not being disposed

Solved this by using smartManagement: SmartManagement.onlyBuilder and initializing screen controllers with GetBuilder

Moriaty911 commented 1 year ago

Try update Getx to 4.6.6. It solved like a magic in my case.

same

lxndrdagreat commented 12 months ago

I was dealing with this issue, which confused me because I had just been using Getx successfully in another app. After comparing all of my code differences, this is what I found, and changing it fixed things for me:

In my broken app, I had changed MaterialApp to GetMaterialApp, but left the following:

routes: {
   HomePage.ROUTE: (context) => const HomePage(),
   HostPage.ROUTE: (context) => const HostPage(),
   ClientPage.ROUTE: (context) => const ClientPage()
},

I changed this to use GetPages (like my previous app):

getPages: [
        GetPage(name: HomePage.ROUTE, page: () => const HomePage()),
        GetPage(name: HostPage.ROUTE, page: () => const HostPage()),
        GetPage(name: ClientPage.ROUTE, page: () => const ClientPage()),
      ]

And the issue was immediately resolved. Controllers are now being disposed/deleted.