Skip to content
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

Document the lock helds during callbacks #346

Closed
jbwdevries opened this issue Dec 15, 2018 · 3 comments
Closed

Document the lock helds during callbacks #346

jbwdevries opened this issue Dec 15, 2018 · 3 comments
Labels
Status: Completed Nothing further to be done with this issue, it can be closed by the requestor or committer. Type: Bug
Milestone

Comments

@jbwdevries
Copy link

During callbacks, the mqtt client holds a lock. If your application also uses locks, this can result in a deadlock. I think it would be best if there was a short section on the README or such that makes a note of this.

@PierreF
Copy link
Contributor

PierreF commented Dec 16, 2018

I don't think we should expose in any way our internal lock behavior. I think it should not impact user and not be "documented" to allow it to change.
Could you describe the use-case where you create a deadlock, so we could fix this bug ?
I also sometime ask myself if we should not simplify the locks used in the library. I count 9 locks, which seems way too much.

@jbwdevries
Copy link
Author

In our case, we had a data class which uses a lock to protect it's data. In on_message, we would call a function in this class. Sometimes, a second thread would call upon this data class to publish messages. This could causes a deadlock, since the on_message would first claim the paho lock, and then our lock, but the other thread would claim them the other way around.

Once we found this, it was easy enough to move the publish call outside the lock, and the problem was solved. Though it took some digging before we actually found out this was the case.

I also wondered why you claim a lock during the callback at all; is that strictly necessary?

@ralight
Copy link
Contributor

ralight commented Aug 16, 2021

In the 1.6.x branch locking in callbacks has been improved so the only callback held is in_callback_mutex, which will not call deadlocks with any client code, so I think this can be closed.

@ralight ralight closed this as completed Aug 16, 2021
@ralight ralight added Status: Completed Nothing further to be done with this issue, it can be closed by the requestor or committer. Type: Bug labels Aug 16, 2021
@ralight ralight added this to the 1.6.0 milestone Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Completed Nothing further to be done with this issue, it can be closed by the requestor or committer. Type: Bug
Projects
None yet
Development

No branches or pull requests

3 participants