basic_pub_sub, publish too quick leads to seg fault

[molizuo]

Confirm by changing [ ] to [x] below:

• I've searched for previous similar issues and didn't find any solution

Known Issue

• I'm using ATS data type endpoint: the endpoint should look like <prefix>-ats.iot.<region>.amazonaws.com

Platform/OS/Device
What are you running the sdk on? Ubuntu 16.04

Describe the question
I am modifying the sample file under basic_pub_sub.
I try to publish a string with in a while loop.

The program will core dump and seg fault after 2 or three seconds.
I try to give it a sleep(0.2), sleep(0.5) under the while loop, the program still core dump and seg fault.
Howerver, while I give it a sleep(1) under the while loop, the program will behave normally for at least 30 min.

I am wondering is this sample code structure unable to handle publish frequency quicker than 1HZ.
The error is src/basic-pub-sub': munmap_chunk(): invalid pointer: 0x00007fffffffd468 ***

I used gdb to back trace the program and found out it is related to
#0 0x00007ffff68fe428 in __GI_raise (sig=sig@entry=6)
at ../sysdeps/unix/sysv/linux/raise.c:54
#1 0x00007ffff690002a in __GI_abort () at abort.c:89
#2 0x00007ffff69407ea in __libc_message (do_abort=do_abort@entry=2,
fmt=fmt@entry=0x7ffff6a59ed8 "*** Error in `%s': %s: 0x%s ***\n")
at ../sysdeps/posix/libc_fatal.c:175
#3 0x00007ffff694d698 in malloc_printerr (ar_ptr=0x0, ptr=,
str=0x7ffff6a59f00 "munmap_chunk(): invalid pointer",
action=) at malloc.c:5006
#4 munmap_chunk (p=) at malloc.c:2842
#5 __GI___libc_free (mem=) at malloc.c:2963
#6 0x000000000048bf7b in aws_byte_buf_clean_up ()

Does anyone know why? Thank you very much.
Below is my publish while loop which is basically the sample code. The rest part is exactly same as the sample code under basic_pub_sub

if (connectionCompletedPromise.get_future().get())
{
while(1)
{
String input = "hi";
ByteBuf payload = ByteBufNewCopy(DefaultAllocator(), (const uint8_t *)input.data(), input.length());
ByteBuf *payloadPtr = &payload;

        auto onPublishComplete = [payloadPtr](Mqtt::MqttConnection &, uint16_t packetId, int errorCode) {
            aws_byte_buf_clean_up(payloadPtr);

            if (packetId)
            {
                fprintf(stdout, "Operation on packetId %d Succeeded\n", packetId);
            }
            else
            {
                fprintf(stdout, "Operation failed with error %s\n", aws_error_debug_str(errorCode));
            }
        };
        connection->Publish(publish_topic.c_str(), AWS_MQTT_QOS_AT_LEAST_ONCE, false, payload, onPublishComplete);
    }
}

[TingDaoK]

Hi @molizuo

The problem here is aws_byte_buf_clean_up(payloadPtr); will clean up the payloadPtr you made when the callback invoked, instead of the one you passed into the publish. connection->Publish right now is not taking over the ownership of payload string from caller. Caller will need to clean it up at the right time, which makes the implementation difficult... We will consider the change to make users life easy.

For now, a workaround I suggest is waiting for the previous publish completes before you start the next publish.

        while (1)
        {
            String input = "hi";
            ByteBuf payload = ByteBufNewCopy(DefaultAllocator(), (const uint8_t *)input.data(), input.length());
            ByteBuf *payloadPtr = &payload;

            std::promise<void> publishPromise;
            auto onPublishComplete = [payloadPtr,
                                      &publishPromise](Mqtt::MqttConnection &, uint16_t packetId, int errorCode) {
                aws_byte_buf_clean_up(payloadPtr);
                if (packetId)
                {
                    fprintf(stdout, "Operation on packetId %d Succeeded\n", packetId);
                }
                else
                {
                    fprintf(stdout, "Operation failed with error %s\n", aws_error_debug_str(errorCode));
                }
                publishPromise.set_value();
            };
            connection->Publish(topic.c_str(), AWS_MQTT_QOS_AT_LEAST_ONCE, false, payload, onPublishComplete);
            publishPromise.get_future().wait();//wait for the previous publish to complete
        }

[TingDaoK]

Alternative approach. If you still want to do multiple publish at the same time, I'll suggest keeping a map from packet_id to the payload point. When the onPublishComplete invoked, look for the right payload and clean it up. BTW, connection->Publish will return the packet_id client will send.

[molizuo]

Thx

[jlong29]

Are there any updates on improving this implementation weakness? This is a major source of low reliability. In the least the samples should be updated to demonstrate the preferred workaround with clear comments regarding what is being done and why.

[psiphi75]

For anyone who gets here and is like ¯_(ツ)_/¯. I made a comment here: #88 (comment)