-
Notifications
You must be signed in to change notification settings - Fork 720
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
Suggestion: Add the ability to call a user supplied method at regular intervals in loop_forever() #787
Comments
Fixing thread-safety is something we want. If thread-safety is fixed, I don't think such callback are still needed. I would prefer avoiding to add more callback / way to use the library as this add maintenance works. |
If you can fix thread safety then using start_loop() to run the client and doing any other updates in the main thread would be viable. The use cases I am considering would be ones where the message received callbacks update data structures but some code that needs to run at a regular interval then uses that data. |
But in your case the thread safety issue is not on paho library side, but on your data structure. You should take a lock before updating the data structure and a lock before the other code uses that data. Even if we fix all thread-safety on paho, it won't solve the possible data race if you don't lock access to your data structure. |
And the need for the user to lock their data correctly is what I meant with the original comment that it can be tricky to ensure the code using loop_start() is thread safe. For the application I'm currently writing, the data structures involved are trivial so I could lock them easily but because writing thread safe code and testing for thread safety isn't always easy having a mechanism that avoids the need for threads can make the paho users life easier. The right place to strike the balance between making life easier for users and keeping the library small enough that you have the resources to maintain it is something I'm not in a position to judge but a library that is maintained and takes a bit more work to use is preferable to one that isn't maintained so if you think this idea isn't worth the effort then that's fine by me. |
But the user supplied method will only solve simple lock needs. One that only need a single lock which aren't tricky. Or do you have a use-case where the callback method solve a problem that single lock don't solve ? I agree that if multiple lock are involved locking become tricky, but then the callback method won't help use either (because if you required multiple lock, it means multiple thing had to run concurrently - or a single lock would be enough - and the callback solution is a full single thread solution). With this solution, I fear that it only fit a too small use-case that could be solve by just using a single lock and adding Does some other language paho.mqtt implementation had such feature ? |
My concern with locking is more about making sure that the callbacks are using the same lock as the main thread and not a copy of it rather than coping with scenarios needing multiple locks. There are plenty of examples posted of users struggling to ensure that their userdata is actually updating the original and not a copy but in that case the behaviour is obviously wrong. Testing that you actually have the correct lock if you expect lock contention to be rare requires some care. If I'm writing code using an event loop it is more typically either X11 or Windows both of which provide timer callbacks (XtAppAddTimeout() or SetTimer() respectively). I couldn't find a paho MQTT implementation that includes one but did find https://docs.hivemq.com/hivemq/3.4/plugins/callbacks.html#scheduled-callback-execution as an example of MQTT implementation that does. If I hadn't seen the comment that I can't find again about locking issues with loop_start() I'd probably have just used that. |
Given that use of loop() is discouraged (and reading the code for loop_forever I can see why) and that it can be tricky to ensure use of loop_start() is thread safe, it would be helpful for loop_forever() to accept an optional argument of a function that, if provided is called regularly to perform any additional functionality that the program needs.
From my limited understanding of the code it looks as though this could be achieved by testing if the function exists and if so calling it just after the call to loop() in loop_forever()
The text was updated successfully, but these errors were encountered: