emqx / CocoaMQTT

MQTT 5.0 client library for iOS and macOS written in Swift
https://www.emqx.com/en
Other
1.59k stars 418 forks source link

Memory build up with sendingMessages dictionary #432

Open abhinavsingh89 opened 2 years ago

abhinavsingh89 commented 2 years ago

Using 1.2.5 release and observed the following implementation below.

You're building up this dictionary but never clearing this out. There is no access to clean this as well. In my case, I was sending a message every 10 seconds which led to substantial memory growth, and eventually, the app ran out of memory. The only way to clean this is up is dealloacting the existing CocoaMQTT object and creating a new one. So essentially recreating the object and the connection.

Screen Shot 2022-01-15 at 11 49 43 PM

The id you're assigning is UInt16, so you can essentially hold 65535 objects. Ideally, as soon as the message is published, it should be removed from this dictionary.

leeway1208 commented 2 years ago

hi @abhinavsingh89 If you use MQTT3.1.1. We highly recommend you to use tag(1.3.0-rc.2). I see my colleague has already done something like "self.sendingMessages.removeValue(forKey: msgid) " in this version. If you still have some problem. Please let me know. Thanks.

abhinavsingh89 commented 2 years ago

Thanks @leeway1208. We would have to run our tests extensively with 1.3.0-rc2. Although even in that version the code says something like this guard deliver.add(frame) else { delegateQueue.async { self.sendingMessages.removeValue(forKey: msgid) } return -1 }

So this will only trigger if you're unable to add to the CocoaMQTTDeliver object which will not be the case unless mqueue in CocoaMQTTDeliver is full. If the system is working normally and messages are being published, messageQueue will be empty and the above will never trigger. This logic only makes sense when mqueue is full. For normal cases when messages are published, it should remove the value from this sendingMessages dic straight away.