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

Set xioHandle to NULL when disconnected #18

Merged

Conversation

villepalo
Copy link
Contributor

@villepalo villepalo commented Apr 18, 2018

mqtt_client->xioHandle is set in mqtt_client_connect (connection state doesn't matter), so I think it is reasonable to assume it will be reset in mqtt_client_disconnect even if the client isn't connected.

Currently this is causing a segfault in iot-sdk-c library (Azure/azure-iot-sdk-c#446). One could argue, that mqtt_client is not used correctly there, but still I think xioHandle should set to NULL event if clientConnected is false.

@pboettch
Copy link

I strongly feel that a unit-test which triggers the problem should be added with such a fix. I had a look at the mqtt_client_ut-part, but I was unable to figure out how to write such a test.

@villepalo
Copy link
Contributor Author

Test case was a good idea. I realized that there was a requirement for this:
"If the client is disconnected, mqtt_client_dowork shall do nothing."

@pboettch
Copy link

And this test-case triggers the problem if xioHandle is not set to NULL?

@villepalo
Copy link
Contributor Author

villepalo commented Apr 19, 2018

This test case reveals that xio_dowork gets called from mqtt_client_dowork, if xioHandle is not set to null, and thus do not meet the requirement ("If the client is disconnected, mqtt_client_dowork shall do nothing").

@jebrando
Copy link
Contributor

I shall pull in the change. Thank you for the PR.

@az-iot-builder-01 az-iot-builder-01 merged commit 8bb623d into Azure:master Apr 23, 2018
@pboettch
Copy link

Will this submodule be updated in the iot-sdk-c as well? Ideally before the next tag.

@villepalo
Copy link
Contributor Author

@jebrando I'm afraid that the segfault occurs again after commit 304f381. Actuall the test case mqtt_client_dowork_does_nothing_if_disconnected_2 reveals that it will occur. Isn't so that the test case proves xio_dowork gets called even if client is disconnected? That is, xioHandle is not null and it may lead to a segfault.

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.

4 participants