eclipse / paho.mqtt.python

paho.mqtt.python
Other
2.19k stars 722 forks source link

Member-object of Client-class keeps reference to the containing object, therefore preventing the garbage-collector from cleaning up the containing object. #812

Closed vitschwilk closed 7 months ago

vitschwilk commented 7 months ago

Bug Description

I wrote a wrapper class for the mqtt client creation to publish data once in a while. The call is used as an object inside another class. This containing class is only used when needed therefor the object will be created inside a function scope. When exiting the function, I assumed the garbage-collector (gc) deletes the object. This does not happen.

After exiting the function, I still see references to this class in the gc. Therefore __del__() of my wrapper-class is never called and the loop is not stopped.

Reproduction

wrapper class:

import gc
import time
import paho.mqtt.client as mqtt

class MqttWrapper:
    def __init__(self, client_name: str, host="127.0.0.1", port=1883):
        self.connected = None
        self._host = host
        self._port = port
        self._client_name = client_name

        self.client = mqtt.Client(self._client_name)
        self.client.username_pw_set("root", "rootpassword")
        self.client.on_connect = self._on_connect
        self.client.on_disconnect = self._on_disconnect
        self.client.on_publish = self._callback_on_publish
        self.client.connect(self._host, self._port)
        self.client.loop_start()
        print("DEBUG: mqtt created")
        self._wait_for_connection()

    def __del__(self):
        print("destructor of MqttWrapper")
        self.client.loop_stop()

    def _wait_for_connection(self, interval=0.1, timeout=5.0):
        '''wait for a connection to be established within 5 s'''
        start = now = time.time()
        connected = self.client.is_connected()

        while not connected and (now - start) < timeout:
            time.sleep(interval)
            connected = self.client.is_connected()
            now = time.time()

        if not connected:
            print(f"WARNING: MQTT didn't connect within {timeout} s timeout")
            self.connected = False
        else:
            print(f"DEBUG: MQTT connected after {(now - start)} s", )
            self.connected = True

    def _callback_on_publish(self, client, userdata, mid):
        print("DEBUG: data published ")

    def _on_connect(self, client, userdata, flags, rc):
        if rc == 0:
            print(f"INFO: Connecting to MQTT Broker! on {self._host}:{self._port}")
        else:
            print(f"ERROR: Failed to connect, return code {rc}='{mqtt.connack_string(rc)}'")
            self.connected = False

    def _on_disconnect(self, client, userdata, rc):
        if rc != 0:
            print(f"WARNING: Mqtt client '{client}' disconnected")
        self.connected = False

    def subscribe(self, topic: str, *args, **kwargs):
        self.client.subscribe(topic, *args, **kwargs)

    def publish(self, topic: str, payload: str, *args, **kwargs):
        self.client.publish(topic, payload, *args, **kwargs)

usage of the wrapper class:

def get_gc(st=""):
    print("<######################>",st)
    for obj in gc.get_objects():
        if isinstance(obj, MqttWrapper):
            print(obj)
            print(gc.get_referents(obj))

def client1():
    get_gc("in c1")
    test_client1 = MqttWrapper("test_client1")
    test_client1.publish("/test/topic", "import")
    get_gc("in c1 after test client")
    time.sleep(2) # wait for publish to be processed

def client2():
    get_gc("in c2")
    test_client2 = MqttWrapper("test_client2")
    test_client2.publish("/test/topic", "import")
    get_gc("in c2 after test client")

if __name__ == "__main__":
    client1()
    get_gc("post c1; pre c2")
    client2()
    get_gc("post c2")
    print("finish")

print("end of script")

This leads to the following output:

<######################> in c1
INFO: Connecting to MQTT Broker! on 127.0.0.1:1883
DEBUG: mqtt created
DEBUG: MQTT connected after 0.0 s
<######################> in c1 after test client
<__main__.MqttWrapper object at 0x7f29259ebc70>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client1', 'client': <paho.mqtt.client.Client object at 0x7f29259ebc10>}, <class '__main__.MqttWrapper'>]
DEBUG: data published 
<######################> post c1; pre c2
<__main__.MqttWrapper object at 0x7f29259ebc70>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client1', 'client': <paho.mqtt.client.Client object at 0x7f29259ebc10>}, <class '__main__.MqttWrapper'>]
<######################> in c2
<__main__.MqttWrapper object at 0x7f29259ebc70>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client1', 'client': <paho.mqtt.client.Client object at 0x7f29259ebc10>}, <class '__main__.MqttWrapper'>]
INFO: Connecting to MQTT Broker! on 127.0.0.1:1883
DEBUG: mqtt created
DEBUG: MQTT connected after 0.0 s
<######################> in c2 after test client
<__main__.MqttWrapper object at 0x7f29252521a0>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client2', 'client': <paho.mqtt.client.Client object at 0x7f2925252110>}, <class '__main__.MqttWrapper'>]
<__main__.MqttWrapper object at 0x7f29259ebc70>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client1', 'client': <paho.mqtt.client.Client object at 0x7f29259ebc10>}, <class '__main__.MqttWrapper'>]
<######################> post c2
<__main__.MqttWrapper object at 0x7f29252521a0>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client2', 'client': <paho.mqtt.client.Client object at 0x7f2925252110>}, <class '__main__.MqttWrapper'>]
DEBUG: data published 
<__main__.MqttWrapper object at 0x7f29259ebc70>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client1', 'client': <paho.mqtt.client.Client object at 0x7f29259ebc10>}, <class '__main__.MqttWrapper'>]
finish
end of script

As you can see the object of test-client1 is never deleted. I cannot access it out of scope obviously but it is still in memory (0x7f29259ebc70).

As a workaround, I now stop the loop inside the scrope where the wrapper-object exists and delete the client member of it. Then the gc works as intended. But this is verry easy to forger/not-know because it is verry different from any other python experience and In my opinion an incorrect behaviour.

So by adding:

test_client1.client.loop_stop() 
del test_client1.client
get_gc("in c1 after cleanup  test client")

and removing the self.client.loop_stop() in __del__() to the functions, I get the following output:

<######################> in c1
INFO: Connecting to MQTT Broker! on 127.0.0.1:1883DEBUG: mqtt created
DEBUG: MQTT connected after 0.0 s

<######################> in c1 after test client
<__main__.MqttWrapper object at 0x7f43e054fcd0>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client1', 'client': <paho.mqtt.client.Client object at 0x7f43e054fc70>}, <class '__main__.MqttWrapper'>]
DEBUG: data published 
<######################> in c1 after cleanup  test client
<__main__.MqttWrapper object at 0x7f43e054fcd0>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client1'}, <class '__main__.MqttWrapper'>]
destructor of MqttWrapper
<######################> post c1; pre c2
<######################> in c2
DEBUG: mqtt created
INFO: Connecting to MQTT Broker! on 127.0.0.1:1883
DEBUG: MQTT connected after 0.10019731521606445 s
<######################> in c2 after test client
<__main__.MqttWrapper object at 0x7f43e00e07c0>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client2', 'client': <paho.mqtt.client.Client object at 0x7f43e054fcd0>}, <class '__main__.MqttWrapper'>]
DEBUG: data published 
<######################> in c2 after cleanup  test client
<__main__.MqttWrapper object at 0x7f43e00e07c0>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client2'}, <class '__main__.MqttWrapper'>]
destructor of MqttWrapper
<######################> post c2
finish
end of script

Now only one object exists when creating the second client. In addition the print from __del__() now is also present.

Environment

Logs

mosquitto.log entries when running the script:

1707386521: New client connected from 127.0.0.1 as test_client2 (p2, c1, k60, u'root').
1707386521: Socket error on client test_client2, disconnecting.
1707386581: New connection from 127.0.0.1 on port 1883.
1707386581: New client connected from 127.0.0.1 as test_client1 (p2, c1, k60, u'root').
1707386583: New connection from 127.0.0.1 on port 1883.

run with enabled logging:

<######################> in c1
2024-02-08 10:12:18,648 - DEBUG - Sending CONNECT (u1, p1, wr0, wq0, wf0, c1, k60) client_id=b'test_client1'
DEBUG: mqtt created
2024-02-08 10:12:18,648 - DEBUG - Received CONNACK (0, 0)
INFO: Connecting to MQTT Broker! on 127.0.0.1:1883
DEBUG: MQTT connected after 0.0 s
2024-02-08 10:12:18,649 - DEBUG - Sending PUBLISH (d0, q0, r0, m1), 'b'/test/topic'', ... (6 bytes)
<######################> in c1 after test client
<__main__.MqttWrapper object at 0x7f027c5c7100>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client1', 'client': <paho.mqtt.client.Client object at 0x7f027bf288b0>}, <class '__main__.MqttWrapper'>]
DEBUG: data published 
<######################> post c1; pre c2
<__main__.MqttWrapper object at 0x7f027c5c7100>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client1', 'client': <paho.mqtt.client.Client object at 0x7f027bf288b0>}, <class '__main__.MqttWrapper'>]
<######################> in c2
<__main__.MqttWrapper object at 0x7f027c5c7100>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client1', 'client': <paho.mqtt.client.Client object at 0x7f027bf288b0>}, <class '__main__.MqttWrapper'>]
2024-02-08 10:12:20,659 - DEBUG - Sending CONNECT (u1, p1, wr0, wq0, wf0, c1, k60) client_id=b'test_client2'
2024-02-08 10:12:20,659 - DEBUG - Sending CONNECT (u1, p1, wr0, wq0, wf0, c1, k60) client_id=b'test_client2'
2024-02-08 10:12:20,660 - DEBUG - Received CONNACK (0, 0)
2024-02-08 10:12:20,660 - DEBUG - Received CONNACK (0, 0)
INFO: Connecting to MQTT Broker! on 127.0.0.1:1883
DEBUG: mqtt created
DEBUG: MQTT connected after 0.0 s
2024-02-08 10:12:20,660 - DEBUG - Sending PUBLISH (d0, q0, r0, m1), 'b'/test/topic'', ... (6 bytes)
2024-02-08 10:12:20,660 - DEBUG - Sending PUBLISH (d0, q0, r0, m1), 'b'/test/topic'', ... (6 bytes)
<######################> in c2 after test client
<__main__.MqttWrapper object at 0x7f027be2a8c0>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client2', 'client': <paho.mqtt.client.Client object at 0x7f027be2a9b0>}, <class '__main__.MqttWrapper'>]
<__main__.MqttWrapper object at 0x7f027c5c7100>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client1', 'client': <paho.mqtt.client.Client object at 0x7f027bf288b0>}, <class '__main__.MqttWrapper'>]
<######################> post c2
<__main__.MqttWrapper object at 0x7f027be2a8c0>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client2', 'client': <paho.mqtt.client.Client object at 0x7f027be2a9b0>}, <class '__main__.MqttWrapper'>]
<__main__.MqttWrapper object at 0x7f027c5c7100>
[{'connected': True, '_host': '127.0.0.1', '_port': 1883, '_client_name': 'test_client1', 'client': <paho.mqtt.client.Client object at 0x7f027bf288b0>}, <class '__main__.MqttWrapper'>]
finish
end of script
PierreF commented 7 months ago

Unless I'm missing something, you use-case isn't supported by Python. Relying on GC to cleanup resource isn't supported by Python AFAIK (and I believe no language with GC support this).

The GC do not guarantee that object get deleted immediately when they lost a reference (your function exit). CPython is a bit special as it do it immediately in some case (when there is no circular reference and paho-mqtt make no promise on circular reference).

If you want to ensure any resource get cleanup, you have to cleanup them at the location you want to cleanup them. Usually using with context (e.g. not fileobj = open("filename") but with open("filename") as fileobj:). But using a finally work well also

So either make your wrapper class usable as with-statement context or add a finally to cleanup resource.

vitschwilk commented 7 months ago

Hi, thank you for your timely answer. That is odd to not support. I don't understand how this client object is different from any other member in my wrapper-class. How can a member object keep the containing object alive?

I do agree that the GC does not "need" to clean up immediately. My issue is, I'm using the same approach in an open-api-backend. Therefore, my python never exits the __main__.py-"script". After running a while and accessing some routs that use this wrapper class, I noticed that the threads from mqtt are never closed. This should have happened in my __dell__ method that is never called.

I previously did try using the class with a context manager, but did not fully delete the client object. I tried again and got it working. For the purpose of searchability, here is the code snipped:

    def __enter__(self):
        return self

    def __exit__(self, exc_type, exc_val, exc_tb):
        print("exiting")
        self.client.loop_stop()

    def __del__(self):
        print("destructor of MqttWrapper")
        if self.client:
            self.client.loop_stop()
PierreF commented 7 months ago

I'm closing the issue as I believe the problem is solved by last comment. Feel free to reopen if that not the case.

I don't understand how this client object is different from any other member in my wrapper-class. How can a member object keep the containing object alive?

The paho-mqtt Client (the value of field self.client in your wrapper class) is still referenced by the thread started by loop_start (that thread run the paho Client). The paho Client have reference to the method _callback_on_publish (thought on_publish). The method had reference to its instance. Therefor even if you no longer have any reference to your wrapper class, it's still hold by this chain: paho thread -> paho Client -> on_publish -> the wrapper class. The "issue" is the presence of callback (unavoidable with paho-mqtt).