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

Add mutex protection to MqttClient_NetDisconnect #342

Merged
merged 1 commit into from Jul 19, 2023

Conversation

embhorn
Copy link
Member

@embhorn embhorn commented Jul 17, 2023

Adds mutex protection and cancels all pending messages in MqttClient_NetDisconnect

Fixes zd16343

Customer confirms fix.

@embhorn embhorn self-assigned this Jul 17, 2023
@embhorn embhorn requested a review from lealem47 July 17, 2023 14:22
@embhorn
Copy link
Member Author

embhorn commented Jul 17, 2023

No idea why the macOS Build Test is failing

Copy link
Contributor

@lealem47 lealem47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

macOS test failure looks like a true positive. Was able to produce locally, the multithreaded test just hangs. Doesn't happen on master.


#ifdef WOLFMQTT_MULTITHREAD
/* Get client lock on to ensure no other threads are active */
wm_SemLock(&client->lockClient);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgarske - I removed the locks for send and recv because they should not be required for modifying the pending response list. Also was causing a failure in the MacOS CI test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we know the socket isn't busy in another thread, or do we let the OS handle that socket sig?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From this API, we are not doing IO directly, so it's okay if other threads are using the socket.

@embhorn embhorn requested a review from lealem47 July 19, 2023 20:05
@lealem47 lealem47 merged commit 65a88f9 into wolfSSL:master Jul 19, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants