Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [paho-dev] Paho c client questions

Hello Ian,

My comments are inline.

Many thanks,
Jürgen

> -----Original Message-----
> From: paho-dev-bounces@xxxxxxxxxxx [mailto:paho-dev-
> bounces@xxxxxxxxxxx] On Behalf Of Ian Craggs
> Sent: Mittwoch, 10. September 2014 12:21
> To: paho-dev@xxxxxxxxxxx
> Subject: Re: [paho-dev] Paho c client questions
> 
> Hi Juergen,
> 
> thanks for your comments.
> 
> 1)  Interesting.   We should have a bug to consider this one.

I'll create a bug for that.

> 2) NULL checks for malloc are handled in Heap.c.  The malloc call is
> overridden.  This will result in error messages in the trace. 

Looking into Heap.c reveals that this code is aware of OOM situations and reacts appropriately.
So Heap.c may return NULL to the MQTTAsync.c where no check happens.

> However,
> the question is what should the action be in the case of malloc
> failure?   I guess the best solution is that the API call returns with
> failure - memory error.  

Agreed on API level.
For code running in background threads it will be more complicated since it has potentially to report the OOM situation to the API consumer.
And if this paths also require memory then they need to be changed.

> However that would add considerable complexity
> and code for handling the error and cleaning up partially allocated
> memory structures.  
Yes. OOM handling is IMO part of the essential complexity of any C/C++ component.

> The Paho embedded clients are intended for use in
> really small environments - they do not allocate any heap memory.
OK. Is the current c client targeted for domains where it is possible to compute upfront an upper boundary for memory consumption?
(In this case I would assume that the client is initialized with a fixed buffer and does not use any dynamic heap allocation strategies. This would make this goal transparent.)

The heading "MQTT C Client for Posix and Windows" made me assume that it also targets these domains. 
The current implementation is problematic in arbitrary communication scenarios where unpredictable payloads may lead to segfaults.

Assuming that the c code could get this robustness (whoever that provides), is there a critical code size that invalidates it's consumption in small environments?

> 
> 3) It is intended to be thread safe.
I'll create a BUG since the usage of MQTTProtocol_assignMsgId is not thread save within:
* MQTTAsync_subscribeMany
* MQTTAsync_unsubscribeMany
* MQTTAsync_send

In the MQTTClient.c the calls to MQTTProtocol_assignMsgId are framed by a Thread_lock_mutex.

> 4) Guarantee is a strong word!  But yes, the callbacks should be made.
> If you can get a trace of the situation where the callback is not made,
> that would be ideal.  Trace is controlled by environment variables and
> for the async client, also with API calls.

A trace is attached that documents the following flow:

1. SEND. TOPIC="test",  qos = 1, Payload = "Msg before disconnect. ...", Payload length = 144
2. Disconnect with timeout = 0

Line 152 shows the freeing of the message.

> Ian
> 
> On 09/09/2014 01:27 PM, Denner, Juergen wrote:
> > Hello,
> >
> > I'm currently playing around with the paho c client (current git-master
> branch/2014-09-04 13:06:27) and wrapping it with a C++ layer.
> > I cannot use the existing cpp wrapper since C++11 is (currently) not a valid
> option for me.
> >
> > Consuming the asynchronous client was straightforward and real fun.
> > While implementing the C++ wrapper and an evaluation scenario, the
> following questions came up.
> >
> > 1) MQTTAsync_token
> > There is a race condition that makes the usage of the MQTTAsync_token in
> publish, subscribe, unsubscribe potentially complicated.
> > It is possible that these methods return *after* the onSuccess or onFailure
> callback already has been invoked.
> > This happens if the MQTTAsync_addCommand is already invoked but the
> method (publish, subscribe, unsubscribe) did not return because of the OS
> scheduler.
> > So creating any structure related to the token is potentially too late.
> >
> > A potential option would be that MQTTAsync_responseOptions contains a
> callback that is invoked before the internal
> > MQTTAsync_addCommand is invoked and after the token is known. Just
> thoughts ...
> >
> > [This is correlated to Bug 433871 but with a different aim.]
> >
> > 2) NULL checks
> > There are no NULL checks on malloc. The current implementation will end
> up with segmentation faults on low memory situations.
> > I assume this is not uncommon on small devices.
> >
> > 3) Thread safety of the client not documented.
> > I assume that send must not be used concurrently by multiple threads.
> > The usage of MQTTProtocol_assignMsgId seems not thread save to me. If
> this was never intended than it's just a documentation question.
> >
> > 4) Callback invocation guarantee
> > I currently need to do a cleanup on the provided context for callback
> methods.
> >
> > Unluckily it seems not to be guaranteed that either onSuccess or onFailure
> is invoked. Especially where MQTTAsync_disconnect is invoked immediately
> after a send.
> > This requires bookkeeping for contexts in the wrapping component. Is this
> behavior intended?
> >
> > In my current evaluation, the most striking issue is the context cleanup.
> >
> > In this mail I've tried to document my initial experience and simply
> summarized my findings and questions on first contact with the library. I
> hope this input is beneficial.
> >
> > Best Regards,
> > Jürgen
> >
> > _______________________________________________
> > paho-dev mailing list
> > paho-dev@xxxxxxxxxxx
> > To change your delivery options, retrieve your password, or unsubscribe
> from this list, visit
> > https://dev.eclipse.org/mailman/listinfo/paho-dev
> 
> --
> Ian Craggs
> icraggs@xxxxxxxxxx                 IBM United Kingdom
> Committer on Paho, Mosquitto
> 
> _______________________________________________
> paho-dev mailing list
> paho-dev@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or unsubscribe
> from this list, visit
> https://dev.eclipse.org/mailman/listinfo/paho-dev
20140910 143226.552 (10808) (0)> MQTTAsync_send:2296
20140910 143227.552 (13868)    (3)> isReady:192
20140910 143227.552 (10808)  (1)> UTF8_validateString:155
20140910 143227.552 (13868)    (3)< isReady:197 (0)
20140910 143227.552 (10808)   (2)> UTF8_validate:129
20140910 143227.552 (13868)   (2)< Socket_getReadySocket:282 (0)
20140910 143227.552 (10808)    (3)> UTF8_char_validate:79
20140910 143227.552 (13868)   (2)> MQTTAsync_sleep:300
20140910 143227.552 (10808)    (3)< UTF8_char_validate:113
20140910 143227.552 (10808)    (3)> UTF8_char_validate:79
20140910 143227.552 (10808)    (3)< UTF8_char_validate:113
20140910 143227.552 (10808)    (3)> UTF8_char_validate:79
20140910 143227.552 (10808)    (3)< UTF8_char_validate:113
20140910 143227.552 (10808)    (3)> UTF8_char_validate:79
20140910 143227.552 (10808)    (3)< UTF8_char_validate:113
20140910 143227.552 (10808)   (2)< UTF8_validate:141 (1)
20140910 143227.552 (10808)  (1)< UTF8_validateString:157 (1)
20140910 143227.552 (10808)  (1)> MQTTProtocol_assignMsgId:73
20140910 143227.552 (10808)  (1)< MQTTProtocol_assignMsgId:86 (2)
20140910 143227.552 Allocating 96 bytes in heap at file ..\..\src\MQTTAsync.c line 2312 ptr 0000000000D04CB0

20140910 143227.552 Allocating 16 bytes in heap at file ..\..\src\MQTTProtocolClient.c line 754 ptr 0000000000D6F850

20140910 143227.552 (10808)  (1)> MQTTStrncpy:731
20140910 143227.552 (10808)  (1)< MQTTStrncpy:741
20140910 143227.552 Allocating 144 bytes in heap at file ..\..\src\MQTTAsync.c line 2326 ptr 0000000000D76B90

20140910 143227.552 (10808)  (1)> MQTTAsync_addCommand:732
20140910 143227.552 Allocating 32 bytes in heap at file ..\..\src\LinkedList.c line 93 ptr 0000000000DAE390

20140910 143227.552 (10808)   (2)> Thread_post_sem:315
20140910 143227.552 (10808)   (2)< Thread_post_sem:324 (0)
20140910 143227.552 (10808)  (1)< MQTTAsync_addCommand:763 (0)
20140910 143227.552 (10808) (0)< MQTTAsync_send:2333 (0)
20140910 143227.552 (10808) (0)> MQTTAsync_disconnect1:2072
20140910 143227.552 Allocating 96 bytes in heap at file ..\..\src\MQTTAsync.c line 2085 ptr 0000000000D04D20

20140910 143227.552 (13336)  (1)< Thread_wait_sem:284 (0)
20140910 143227.552 (10808)  (1)> MQTTAsync_addCommand:732
20140910 143227.552 Allocating 32 bytes in heap at file ..\..\src\LinkedList.c line 93 ptr 0000000000DAE270

20140910 143227.552 (13336)  (1)> MQTTAsync_checkTimeouts:1176
20140910 143227.552 (10808)   (2)> Thread_post_sem:315
20140910 143227.552 (13336)  (1)< MQTTAsync_checkTimeouts:1248
20140910 143227.552 (10808)   (2)< Thread_post_sem:324 (0)
20140910 143227.552 (13336)  (1)> MQTTAsync_processCommand:934
20140910 143227.552 Allocating 32 bytes in heap at file ..\..\src\LinkedList.c line 56 ptr 0000000000DAE3F0

20140910 143227.552 Freeing 32 bytes in heap at file ..\..\src\LinkedList.c line 389, heap use now 1408 bytes

20140910 143227.552 Freeing 32 bytes in heap at file ..\..\src\LinkedList.c line 222, heap use now 1376 bytes

20140910 143227.552 Allocating 48 bytes in heap at file ..\..\src\MQTTAsync.c line 1071 ptr 0000000000D62F70

20140910 143227.552 (10808)  (1)< MQTTAsync_addCommand:763 (0)
20140910 143227.552 (13336)   (2)> MQTTProtocol_startPublish:146
20140910 143227.552 Allocating 48 bytes in heap at file ..\..\src\MQTTProtocolClient.c line 172 ptr 0000000000D62FB0

20140910 143227.552 (10808) (0)< MQTTAsync_disconnect1:2100 (0)
20140910 143227.552 (13336)    (3)> MQTTProtocol_createMessage:174
20140910 143227.552 Allocating 32 bytes in heap at file ..\..\src\MQTTProtocolClient.c line 207 ptr 0000000000DAE300

20140910 143227.552 (10808) (0)> MQTTAsync_isConnected:2131
20140910 143227.552 (13336)     (4)> MQTTProtocol_storePublication:209
20140910 143227.552 Allocating 144 bytes in heap at file ..\..\src\MQTTProtocolClient.c line 224 ptr 0000000000D76C30

20140910 143227.552 Allocating 32 bytes in heap at file ..\..\src\LinkedList.c line 93 ptr 0000000000DAE600

20140910 143227.552 (13336)     (4)< MQTTProtocol_storePublication:229
20140910 143227.552 (13336)    (3)< MQTTProtocol_createMessage:194
20140910 143227.552 Allocating 32 bytes in heap at file ..\..\src\LinkedList.c line 93 ptr 0000000000DAE690

20140910 143227.552 (13336)    (3)> MQTTProtocol_startPublishCommon:123
20140910 143227.552 (13336)     (4)> MQTTPacket_send_publish:675
20140910 143227.552 Allocating 16 bytes in heap at file ..\..\src\MQTTPacket.c line 676 ptr 0000000000D6F830

20140910 143227.552 Allocating 16 bytes in heap at file ..\..\src\MQTTPacket.c line 684 ptr 0000000000D6F910

20140910 143227.553 (13336)      (5)> MQTTPacket_sends:224
20140910 143227.553 Allocating 16 bytes in heap at file ..\..\src\MQTTPacket.c line 225 ptr 0000000000D6F930

20140910 143227.553 (13336)       (6)> MQTTPacket_encode:266
20140910 143227.553 (13336)       (6)< MQTTPacket_encode:276 (2)
20140910 143227.553 (13336)       (6)> MQTTPersistence_put:347
20140910 143227.553 (13336)       (6)< MQTTPersistence_put:381 (0)
20140910 143227.553 (13336)       (6)> Socket_putdatas:443
20140910 143227.553 (13336)        (7)> Socket_writev:399
20140910 143227.553 (13336)        (7)< Socket_writev:420 (0)
20140910 143227.553 (13336)       (6)< Socket_putdatas:485 (0)
20140910 143227.553 Freeing 16 bytes in heap at file ..\..\src\MQTTPacket.c line 250, heap use now 1728 bytes

20140910 143227.553 (13336)      (5)< MQTTPacket_sends:251 (0)
20140910 143227.553 Freeing 16 bytes in heap at file ..\..\src\MQTTPacket.c line 695, heap use now 1712 bytes

20140910 143227.553 Freeing 16 bytes in heap at file ..\..\src\MQTTPacket.c line 708, heap use now 1696 bytes

20140910 143227.553 1348  -> PUBLISH msgid: 2 qos: 1 retained: 0 (0) payload: Msg before disconnec
20140910 143227.553 (13336)     (4)< MQTTPacket_send_publish:714 (0)
20140910 143227.553 (13336)    (3)< MQTTProtocol_startPublishCommon:127 (0)
20140910 143227.553 (13336)   (2)< MQTTProtocol_startPublish:157 (0)
20140910 143227.553 Freeing 48 bytes in heap at file ..\..\src\MQTTAsync.c line 1106, heap use now 1680 bytes

20140910 143227.553 Allocating 32 bytes in heap at file ..\..\src\LinkedList.c line 93 ptr 0000000000DAE0C0

20140910 143227.553 (13336)  (1)< MQTTAsync_processCommand:1166
20140910 143227.553 (13336)  (1)> MQTTAsync_processCommand:934
20140910 143227.553 (10808) (0)< MQTTAsync_isConnected:2136 (1)
20140910 143227.553 Allocating 32 bytes in heap at file ..\..\src\LinkedList.c line 56 ptr 0000000000DAE750

20140910 143227.553 Freeing 32 bytes in heap at file ..\..\src\LinkedList.c line 389, heap use now 1696 bytes

20140910 143227.553 Freeing 32 bytes in heap at file ..\..\src\LinkedList.c line 222, heap use now 1664 bytes

20140910 143227.553 (13336)   (2)> MQTTAsync_checkDisconnect:772
20140910 143227.553 (13336)    (3)> MQTTAsync_closeSession:1761
20140910 143227.553 (13336)     (4)> MQTTAsync_closeOnly:1737
20140910 143227.553 (13336)      (5)> MQTTPacket_send_disconnect:462
20140910 143227.553 (13336)       (6)> MQTTPacket_send:178
20140910 143227.553 Allocating 16 bytes in heap at file ..\..\src\MQTTPacket.c line 179 ptr 0000000000D6F830

20140910 143227.553 (13336)        (7)> MQTTPacket_encode:266
20140910 143227.553 (13336)        (7)< MQTTPacket_encode:276 (1)
20140910 143227.553 (13336)        (7)> Socket_putdatas:443
20140910 143227.553 (13336)         (8)> Socket_writev:399
20140910 143227.553 (13336)         (8)< Socket_writev:420 (0)
20140910 143227.553 (13336)        (7)< Socket_putdatas:485 (0)
20140910 143227.553 Freeing 16 bytes in heap at file ..\..\src\MQTTPacket.c line 203, heap use now 1648 bytes

20140910 143227.553 (13336)       (6)< MQTTPacket_send:205 (0)
20140910 143227.553 1348  -> DISCONNECT (0)
20140910 143227.553 (13336)      (5)< MQTTPacket_send_disconnect:467 (0)
20140910 143227.553 (13336)      (5)> Socket_close:548
20140910 143227.553 (13336)       (6)> Socket_close_only:522
20140910 143227.553 (13336)       (6)< Socket_close_only:536 (0)
20140910 143227.553 (13336)       (6)> SocketBuffer_cleanup:126
20140910 143227.553 (13336)       (6)< SocketBuffer_cleanup:134
20140910 143227.553 Freeing 16 bytes in heap at file ..\..\src\LinkedList.c line 219, heap use now 1632 bytes

20140910 143227.553 Freeing 32 bytes in heap at file ..\..\src\LinkedList.c line 222, heap use now 1616 bytes

20140910 143227.553 Removed socket 1348
20140910 143227.553 Reset max fdp1 to 1
20140910 143227.553 (13336)      (5)< Socket_close:574
20140910 143227.553 (13336)     (4)< MQTTAsync_closeOnly:1755
20140910 143227.553 (13336)     (4)> MQTTAsync_cleanSession:1789
20140910 143227.553 (13336)      (5)> MQTTPersistence_clear:156
20140910 143227.553 (13336)      (5)< MQTTPersistence_clear:160 (0)
20140910 143227.553 (13336)      (5)> MQTTProtocol_emptyMessageList:694
20140910 143227.553 (13336)      (5)< MQTTProtocol_emptyMessageList:701
20140910 143227.553 (13336)      (5)> MQTTProtocol_emptyMessageList:694
20140910 143227.553 (13336)       (6)> MQTTProtocol_removePublication:239
20140910 143227.553 Freeing 144 bytes in heap at file ..\..\src\MQTTProtocolClient.c line 242, heap use now 1584 bytes

20140910 143227.553 Freeing 16 bytes in heap at file ..\..\src\MQTTProtocolClient.c line 243, heap use now 1440 bytes

20140910 143227.553 Freeing 32 bytes in heap at file ..\..\src\LinkedList.c line 219, heap use now 1424 bytes

20140910 143227.553 Freeing 32 bytes in heap at file ..\..\src\LinkedList.c line 222, heap use now 1392 bytes

20140910 143227.553 (13336)       (6)< MQTTProtocol_removePublication:246
20140910 143227.553 Freeing 48 bytes in heap at file ..\..\src\LinkedList.c line 358, heap use now 1360 bytes

20140910 143227.553 Freeing 32 bytes in heap at file ..\..\src\LinkedList.c line 360, heap use now 1312 bytes

20140910 143227.553 (13336)      (5)< MQTTProtocol_emptyMessageList:701
20140910 143227.553 (13336)      (5)> MQTTAsync_emptyMessageQueue:1283
20140910 143227.553 (13336)      (5)< MQTTAsync_emptyMessageQueue:1297
20140910 143227.553 (13336)      (5)> MQTTAsync_removeResponsesAndCommands:1307
20140910 143227.553 Freeing 144 bytes in heap at file ..\..\src\MQTTAsync.c line 863, heap use now 1280 bytes

20140910 143227.553 Freeing 96 bytes in heap at file ..\..\src\LinkedList.c line 358, heap use now 1136 bytes

20140910 143227.553 Freeing 32 bytes in heap at file ..\..\src\LinkedList.c line 360, heap use now 1040 bytes

20140910 143227.553 1 responses removed for client 
20140910 143227.553 0 commands removed for client 
20140910 143227.553 (13336)      (5)< MQTTAsync_removeResponsesAndCommands:1339
20140910 143227.553 (13336)     (4)< MQTTAsync_cleanSession:1805 (0)
20140910 143227.553 (13336)    (3)< MQTTAsync_closeSession:1767
20140910 143227.553 Calling disconnect complete for client 
20140910 143227.553 (13336)   (2)< MQTTAsync_checkDisconnect:789
20140910 143227.553 Freeing 96 bytes in heap at file ..\..\src\MQTTAsync.c line 870, heap use now 1008 bytes

20140910 143227.553 (13336)  (1)< MQTTAsync_processCommand:1166
20140910 143227.553 (13336)  (1)> Thread_wait_sem:263
20140910 143227.553 (13336)  (1)< Thread_wait_sem:284 (0)
20140910 143227.553 (13336)  (1)> MQTTAsync_checkTimeouts:1176
20140910 143227.553 (13336)  (1)< MQTTAsync_checkTimeouts:1248
20140910 143227.553 (13336)  (1)> Thread_wait_sem:263
20140910 143227.553 (13868)   (2)< MQTTAsync_sleep:306
20140910 143227.553 (13868)   (2)> MQTTAsync_retry:2369
20140910 143227.553 (13868)    (3)> MQTTProtocol_retry:624
20140910 143227.553 (13868)    (3)< MQTTProtocol_retry:643
20140910 143227.553 (13868)   (2)< MQTTAsync_retry:2379
20140910 143227.553 (13868)  (1)< MQTTAsync_cycle:2633 (-1)
20140910 143227.553 (13868)  (1)> MQTTAsync_disconnect1:2072
20140910 143227.553 (13868)  (1)< MQTTAsync_disconnect1:2100 (-3)
20140910 143227.553 (13868)  (1)> MQTTAsync_cycle:2499

Back to the top