vukoye / xmpp_dart

Lightweight XMPP client library written in Dart
Apache License 2.0
83 stars 64 forks source link

Connections cannot be garbage collected #59

Closed ol88 closed 3 years ago

ol88 commented 3 years ago

Hi Vukoye and thanks for the great package!

In my app, I have a need to re-create a Connection frequently with updated user credentials, this exposed a behavior which I think may be irrelevant in some use cases but undesirable in others: Connections cannot be garbage collected even after being closed and de-referenced by anything other than the xmpp_stone package itself.

Since a connection creates various instances of PresenceManager, Ping Manager, etc... and those instances are stored in static maps within those objects with the parent Connection instance being the key, a connection is always referenced by the package itself while instances of the various Manager objects are stored in those static maps.

Also the various Manager objects do not provide any static method so far to remove instances from the static instances maps.

To reproduce, simply create and close a Connection multiple times using buttons for example, giving enough time to create those Connections.

final someCredentials = XmppAccountSettings(<your account details here>);
Connection? myConnection;

@override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: const Text('Example')),
      body: Container(
          child: Column(
        children: [
          ElevatedButton(
            onPressed: () {
              setState(() {
                myConnection = Connection(someCredentials);
              });
            },
            child: Text('Create XMPP Connection'),
          ),
          ElevatedButton(
            onPressed: () {
              myConnection?.close();
            },
            child: Text('Close XMPP Connection'),
          ),
        ],
      )),
    );
  }

After a few connections creation and close, user the dart devtools in the memory tab, you can take a heap snapshot and look at the xmpp_stone package. There should be the same amount of connections that the amount of time you clicked on the create button. You can try to manually trigger the garbage collector but those connections will just sit there and never be garbage collected, accumulating as more connections are created.

Connection_memory_leak

Also related is that "closed" connection timers can remain active. I often randomly see heartbeat message being sent (or logged as if they were sent):

[log] ---Xmpp Sending:---
[log] <r xmlns="urn:xmpp:sm:3"/>
[log] ---Xmpp Sending:---
[log] <r xmlns="urn:xmpp:sm:3"/>
[log] ---Xmpp Sending:---
[log] <r xmlns="urn:xmpp:sm:3"/>

In this case I believe it is because the timer in the StreamManagementModule sometimes is not canceled properly when a connection is closed. This is actually what got me on track to look up into all of the above...

I believe a lot of those behaviors come from a typical chat app use case where all connection are opened when the app is opened and are expected to stay active and never closed for the lifecycle of the app. But other use case for XMPP like mine may require a different approach.

I'll open a PR to fix the above issues without affecting the package behavior at all, it'll be a purely optional dispose method added to the Connection which can be used instead of close for those use cases like mine when garbage collection is desirable.