{"action":"closed","issue":{"url":"https://api.github.com/repos/eclipse/mosquitto/issues/112","repository_url":"https://api.github.com/repos/eclipse/mosquitto","labels_url":"https://api.github.com/repos/eclipse/mosquitto/issues/112/labels{/name}","comments_url":"https://api.github.com/repos/eclipse/mosquitto/issues/112/comments","events_url":"https://api.github.com/repos/eclipse/mosquitto/issues/112/events","html_url":"https://github.com/eclipse/mosquitto/issues/112","id":140897125,"number":112,"title":"inflight messages not always decremented for QoS > 0","user":{"login":"ralight","id":79578,"avatar_url":"https://avatars.githubusercontent.com/u/79578?v=3","gravatar_id":"","url":"https://api.github.com/users/ralight","html_url":"https://github.com/ralight","followers_url":"https://api.github.com/users/ralight/followers","following_url":"https://api.github.com/users/ralight/following{/other_user}","gists_url":"https://api.github.com/users/ralight/gists{/gist_id}","starred_url":"https://api.github.com/users/ralight/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ralight/subscriptions","organizations_url":"https://api.github.com/users/ralight/orgs","repos_url":"https://api.github.com/users/ralight/repos","events_url":"https://api.github.com/users/ralight/events{/privacy}","received_events_url":"https://api.github.com/users/ralight/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/eclipse/mosquitto/labels/bug","name":"bug","color":"fc2929"}],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":0,"created_at":"2016-03-15T07:30:52Z","updated_at":"2016-03-15T07:30:53Z","closed_at":"2016-03-15T07:30:53Z","body":"migrated from Bugzilla [#474935](https://bugs.eclipse.org/bugs/show_bug.cgi?id=474935)\nstatus RESOLVED severity *normal* in component *Mosquitto client library* for *1.4*\nReported in version *1.4* on platform *PC*\nAssigned to: Roger Light\n\nOn 2015-08-13 17:24:08 -0400, Joe McIlvain wrote:\n\n> I'm using libmosquitto release 1.4.2, which includes a bugfix where mosq->inflight_messages was not being properly decremented for QoS > 0:\n> https://bugs.eclipse.org/bugs/show_bug.cgi?id=464436\n> \n> However, I am running into issues where the inflight_messages are *usually* decremented, but not always, meaning that eventually my client reaches a kind of permanent inflight saturation deadlock, in which the inflight_messages counter will not be decremented until a response is received, but no response will ever be received because there are no messages \"actually\" in-flight, and no new messages will be sent until the counter is decremented.  I have found out this much using GDB, but am still trying to find more details about what is (not) happening here.\n> \n> This issue reproduces reliably on my machine with the following program flow using a single thread with the synchronous API with libmosquitto compiled with \"WITH_THREADING=no WITH_SRV=no WITH_UUID=no\":\n> \n> 1. Create a new client and connect to a running mosquitto broker.\n> \n> 2. Publish 500 messages at QoS 1 (or 2), capturing the 500 message ids into an array.\n> \n> 3. Run combinations of select, loop_read, loop_write, and loop_misc until the on_publish callback receives all 500 message ids that were captured, clearing the array and confirming that all 500 published messages were successfully delivered to the broker.\n> \n> 4. Sleep for a fraction of a second to throttle the program.\n> \n> 4. Goto 2 and repeat forever.\n> \n> On my machine, this \"protocol-deadlock\"s the client after the first 9500 published messages, every time.  As mentioned, I use the message id passed to the on_publish callback to confirm that all 500 messages are delivered before proceeding to the next batch.  Consequently, there should be no messages in-flight at the start of each batch.  But when checking mosq->inflight_messages with GDB, I find that the number of perceived inflight messages at the start of each batch will grow slowly over the course of the 9500 messages (19 batches) until at the start of the 20th batch, the mosq->inflight_messages counter reads 20 and thus no further progress is possible.\n\nOn 2015-08-13 18:05:34 -0400, Joe McIlvain wrote:\n\n> Okay, I've tracked down the precise cause.\n> \n> These two lines both do the same check to determine whether the message is one that should be immediately sent or considered to be \"in flight\":\n> \n> This one does the check, then raises the counter if it's \"in flight\":\n> http://git.eclipse.org/c/mosquitto/org.eclipse.mosquitto.git/tree/lib/messages_mosq.c?id=SHA: 22fee31ba42a2cae272cec1547010045916fa15b#n124\n> \n> Then this one does the same check, then sends the publish immediately if it's considered to be \"in flight\"\n> http://git.eclipse.org/c/mosquitto/org.eclipse.mosquitto.git/tree/lib/mosquitto.c?id=SHA: 22fee31ba42a2cae272cec1547010045916fa15b#n598\n> \n> However, the issue here is that the counter is raised between the first and second check, meaning that for the 20th message, the first check will consider it to be \"in flight\" and the second check will consider it to be queued (not sent immediately).\n> \n> Here is one simple diff that fixes the issue, although it may not be the most elegant way to solve the problem:\n> \n> ```\n> *** mosquitto.c\t2015-08-13 15:03:29.268337849 -0700\n> --- patched/mosquitto.c\t2015-08-13 15:03:22.330283257 -0700\n> *************** int mosquitto_publish(struct mosquitto *\n> *** 593,599 ****\n>   \n>   \t\tpthread_mutex_lock(&mosq->out_message_mutex);\n>   \t\t_mosquitto_message_queue(mosq, message, mosq_md_out);\n> ! \t\tif(mosq->max_inflight_messages == 0 || mosq->inflight_messages < mosq->max_inflight_messages){\n>   \t\t\tif(qos == 1){\n>   \t\t\t\tmessage->state = mosq_ms_wait_for_puback;\n>   \t\t\t}else if(qos == 2){\n> --- 593,599 ----\n>   \n>   \t\tpthread_mutex_lock(&mosq->out_message_mutex);\n>   \t\t_mosquitto_message_queue(mosq, message, mosq_md_out);\n> ! \t\tif(mosq->max_inflight_messages == 0 || mosq->inflight_messages <= mosq->max_inflight_messages){\n>   \t\t\tif(qos == 1){\n>   \t\t\t\tmessage->state = mosq_ms_wait_for_puback;\n>   \t\t\t}else if(qos == 2){\n> ```\n> \n> I'd suggest instead of the above patch that the flow be amended to only perform this check in one place instead of both, possibly using a return value from _mosquitto_message_queue to indicate whether it should be considered queued or in flight.\n\nOn 2015-08-13 19:44:47 -0400, Roger Light wrote:\n\n> Thanks for the detailed report, I think I agree with your analysis. I also agree that using a return value from queue() would be better than repeating the check, I'll see about getting that sorted in the next day or so.\n\nOn 2015-08-17 16:14:34 -0400, Roger Light wrote:\n\n> Thanks very much, I've now submitted a fix for this.\n\nOn 2015-09-01 14:36:27 -0400, Joe McIlvain wrote:\n\n> Ah, great! I somehow missed the email when this was fixed and was about to check back in about this.\n> \n> Thanks for the quick resolution and for the version bump!\n\n"}}