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

Suspected bug with MQTTYield() / cycle() #223

Open
diewelt opened this issue Jul 12, 2021 · 0 comments
Open

Suspected bug with MQTTYield() / cycle() #223

diewelt opened this issue Jul 12, 2021 · 0 comments
Labels
fix added A fix has been added, but not yet in a release
Milestone

Comments

@diewelt
Copy link

diewelt commented Jul 12, 2021

I experienced a weird issue with MQTTClient-C. Without an explicit error MQTTYield() returns error on both, IoT board and host subscriber/publisher. I used Paho MQTT library for my board and host application.

Socket has to be closed if an error is returned while reading or writing in my thought, because it is already closed or something wrong with the socket. And it's clearer when we check cycle() returns MQTTCLNT_FAILURE(== -1) on all failures.

Here is the scenario where MQTTYield() returns an error without an explicit error.

MQTTYield() is given 1 sec and readPacket() spends 990 ms. And then context switching occurs. The other task consumes the remaing 10 ms for the timer. Now sendPacket() is called. Timer is already expired at the point when sendPacket() is called. So sendPacket() returns an MQTTCLNT_FAILURE, cycle() returns MQTTCLNT_FAILURE, and then MQTTYield() returns MQTTCLNT_FAILURE.

`int cycle(MQTTClient* c, Timer* timer)
{
int packet_type = readPacket(c, timer);

switch (packet_type)
{

---- snip ---

context switching (for example)
timer expired here

---- snip ---
case PUBLISH:
rc = sendPacket(c, len, timer);
if(rc == MQTTCLNT_FAILURE)
goto exit; // there was a problem
break;
---- snip ---
}

return rc;

}`

In this scenario, the application has to close the socket() without an actual error. In my setup this happens too often(several tens of times a day).

The simplest solution is to give a reasonable time to sendPacket() irregardless of the remaining time after readPacket(). It's safe enough because sendPacket() is executed within certain bounded time in most systems.

`
int cycle(MQTTClient* c, Timer* timer)
{
Timer resonableConstantTimer;
int packet_type = readPacket(c, timer);

TimerInit(&resonableConstantTimer);
TimerCountdownMS(&resonableConstantTimer, RESONABLE_CONSTANT_TIME);

switch (packet_type)
{

---- snip ---
case PUBLISH:
rc = sendPacket(c, len, resonableConstantTimer);
if(rc == MQTTCLNT_FAILURE)
goto exit; // there was a problem
break;
---- snip ---
}

return rc;

}
`

Kyle Shim

vinhlq added a commit to vinhlq/paho.mqtt.embedded-c that referenced this issue Nov 21, 2021
- In the sendPacket() function, when timer is expired mqttwrite() function is not excute. So to give it a change to send ack packet (issue eclipse#223), we need to define a minimal timeout
- The send packet timeout is defined by MIN_SEND_PACKET_TIMEOUT_MS macro
@vinhlq vinhlq mentioned this issue Nov 21, 2021
@icraggs icraggs added this to the 1.2 milestone Jul 26, 2023
@icraggs icraggs added the fix added A fix has been added, but not yet in a release label Aug 1, 2023
CIPop pushed a commit to CIPop/paho.mqtt.embedded-c that referenced this issue Aug 3, 2023
CIPop pushed a commit to CIPop/paho.mqtt.embedded-c that referenced this issue Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix added A fix has been added, but not yet in a release
Projects
None yet
Development

No branches or pull requests

2 participants