CC3135: Error handling Https Client

The looks ok although i would call the disconnect each time the connection was established successfully (even if the SendRequest returns error).

You can try to send couple of requests without connecting/disconnecting however most web servers use relatively short watchdog (idle) timeout for the connections and even 10 seconds may be enough for them to close the connection.

Hi Kobi,

Thanks for the reply. I have the following questions:

1. If the server is not running, I get SL_ERROR_BSD_ECONNREFUSED as expected when HTTPClient_connect is called. As in the pseudo code, I destroy the client and recreate it everytime this happens. Over a period of time, I start getting SL_RET_CODE_MALLOC_ERROR for HTTPClient_connect function. This frequent creation and destruction of client causes us to run out of memory and can potentially lead to memory fragmentation issues.

On further analysis, found that HTTPClient_create and HTTPClient_connect functions using malloc functions. How does using static memory management i.e. commenting SL_MEMORY_MGMT_DYNAMIC in user.h, lead to malloc errors?

Also, restarting the NWP using sl_Start and sl_Stop did not help resolve the SL_RET_CODE_MALLOC_ERROR. The only way to resolve the error was to do a power reset.

2. Internally whenever HTTPClient_connect fails, HTTPClient_disconnect is called. Then is there any reason why we need to do it explicitly?

The "user.h" is for the simplelink driver. Static memory is not recommended (it will cause issues specifically when calling SL commands from the SL event handlers).

The networking libraries in the SDK (such as httpclient) are using dynamic memory.

We are not aware of memory leaks and it may relate to the application usage.

Ofcourse, you can replace the httpclient with any other http implementation that uses static memory (on top to Sl Sockets or SlNetSock). 

I can confirm that our application does not use any dynamic memory during run time. All the dynamic memory allocation for FreeRTOS is done only during initialization. The only dynamic memory allocation during run time is done by the HTTPClient functions.

We commented out the httpclient connect , transfer calls I.e calls made to simplelink API’s keeping all other application code intact, printed the heap stats at regular intervals and confirmed that heap is always constant, I.e no alloc and dealloc during run time. 

Explanation of the issue:

1. Scenario 1: Initially we start our application without starting the server. So HTTPClient_Connect returns an error of SL_ERROR_BSD_ECONNREFUSED. As the connect fails, we destroy the client and then retry creating a client and establishing connection again after 10 seconds. HTTPClient_Connect gives the same SL_ERROR_BSD_ECONNREFUSED error. But after 10 hours, HTTPClient_Connect returns SL_RET_CODE_MALLOC_ERROR. The frequent allocation and deallocation of memory leads to running out of memory. We could see that on every cycle of connection and disconnection of HttpClient, it leaks 24 bytes of memory.
   

2. Scenario 2: Start the application without starting the server.HTTPClient_Connect returns an error of SL_ERROR_BSD_ECONNREFUSED. Without destroying the client, retry HTTPClient_Connect. Even without destroying the HttpClient, there is memory leak for every cycle of calling HTTPClient_Connect.
   

3. Scenario 3: After some time run the server. Now HTTPClient_Connect is successful and starts sending data. We can see that even then we keep running out of memory.

Irrespective of successful connection or disconnection, it keeps leaking memory.

Available heap memory in bytes for each of the scenarios:

PS: Plz note the updated code in original question

To analyze further, we added heap statistics before and after every http client call, we found is

Create client consumes 744 bytes of heap.

connect client consumes 24 bytes of heap, In this case the client was not able to make successful connection.

Destroy Client returns the 744 back,

The 24 bytes consumed by connect call adds up cumulatively for every call, till the heap runs out of memory....

we moved the setheader calls to the top and made it one time setting for this test, this is the only difference from the code shared above.

Thanks. We will check this.

Are you using a secure connection (HTTPS)? 

I've briefly tried to create a loop over the sequence in the HttpGet and only showed signs of leak at the first iteration (it looks like 16 bytes that are being allocated in the connect and are not release) but this is not happening on the next iterations (everything is being freed).

I haven't checked with error from the server.

It will take couple of days for us to debug this deeper (basically you have the sources of the http library so you can start before - it is not too complicated).

I'll report when i have more info or a fix.

Yes We are using HTTPS, We will also try to debug further.

We put breakpoints in malloc and free functions of freertos on http_client connect, this is what we found.

HTTPClient_connect->createSecAttribs->SlNetSock_secAttribCreate-> 4 bytes of Allocation.
HTTPClient_connect->createSecAttribs->SlNetSock_secAttribSet-> 16 bytes of Allocation.
HTTPClient_connect->HTTPClient_connect2->HTTPClient_setHeader->16 bytes.
HTTPClient_connect->HTTPClient_connect2->HTTPClient_setHeader->12 bytes.
HTTPClient_connect->HTTPClient_connect2->slNetsock_connect->SlNetSock_create->SlNetSock_AllocVirtualSocket->12 bytes.

Try to connect and connection fails.

HTTPClient_connect->HTTPClient_connect2->HTTPClient_disconnect->SlNetSock_secAttribDelete -> 4 Bytes
HTTPClient_connect->HTTPClient_connect2->HTTPClient_disconnect->SlNetSock_secAttribDelete -> 16 bytes
HTTPClient_connect->HTTPClient_connect2->HTTPClient_disconnect->clearReqHeaders -> 24 Bytes
HTTPClient_connect->HTTPClient_connect2->HTTPClient_disconnect->clearReqHeaders -> 24 Bytes

Connect fail return error code is -111.

This points out to allocation of slnetsock_AllocVirtualSocket is not freed. Hope this will help you.

Thanks. I'll check this and get back to you in couple of days.

Hi,

I couldn't find the issue in the httpclient, but since i only used standard cloud servers, i couldn't cause them to be off.

Have you noticed the memory leak issue when the server is up and running?

If you add this file to your project, it will add relevant malloc/free information.

It uses the Display_printf. You can replace it if you use another printing method as long as the print is not allocating memory by itself.  

/*
 * Copyright (c) 2016-2020 Texas Instruments Incorporated - http://www.ti.com
 * All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
 * are met:
 *
 * *  Redistributions of source code must retain the above copyright
 *    notice, this list of conditions and the following disclaimer.
 *
 * *  Redistributions in binary form must reproduce the above copyright
 *    notice, this list of conditions and the following disclaimer in the
 *    documentation and/or other materials provided with the distribution.
 *
 * *  Neither the name of Texas Instruments Incorporated nor the names of
 *    its contributors may be used to endorse or promote products derived
 *    from this software without specific prior written permission.
 *
 * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
 * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
 * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
 * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
 * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
 * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
 * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
 * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
 * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
 * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
 * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 *
 */

/*
 *  ======== memory.c ========
 */

#if defined(__ti__) && !defined(__clang__)

#pragma FUNC_EXT_CALLED(malloc);
#pragma FUNC_EXT_CALLED(memalign);
#pragma FUNC_EXT_CALLED(free);
#pragma FUNC_EXT_CALLED(calloc);
#pragma FUNC_EXT_CALLED(realloc);
#pragma FUNC_EXT_CALLED(aligned_alloc);

#define ATTRIBUTE

#elif defined(__IAR_SYSTEMS_ICC__)

#define ATTRIBUTE

#else

#define ATTRIBUTE __attribute__ ((used))

#endif

#include <stddef.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>

#include <FreeRTOS.h>
#include <portmacro.h>
#include <ti/display/Display.h>
extern Display_Handle display;


#if defined(__GNUC__) && !defined(__ti__)

#include <reent.h>

#endif

/*
 * Header is a union to make sure that the size is a power of 2.
 *
 * On the MSP430 small model (MSP430X), size_t is 2 bytes, which makes
 * the size of this struct 6 bytes.
 */
typedef union Header {
    struct {
        void *actualBuf;
        size_t size;
    } header;
    int pad[2];        /* 4 words on 28L, 8 bytes on most others */
} Header;

/*
 *  ======== malloc ========
 */
void ATTRIBUTE *malloc(size_t size)
{
    Header *packet;
    size_t allocSize;
    int diff = xPortGetFreeHeapSize();

    allocSize = size + sizeof(Header);

    /*
     * If size is very large and allocSize overflows, the result will be
     * smaller than size. In this case, don't try to allocate.
     */
    if ((size == 0) || (allocSize < size)) {
        errno = EINVAL;
        return (NULL);
    }

    packet = (Header *)pvPortMalloc(allocSize);

    if (packet == NULL) {
        errno = ENOMEM;
        return (NULL);
    }

    packet->header.actualBuf = (void *)packet;
    packet->header.size = allocSize;
    diff = diff  - xPortGetFreeHeapSize();
    Display_printf(display, 0, 0, "[%d] %p = malloc (%d) %d", xPortGetFreeHeapSize(), packet+1, size, diff);

    return (packet + 1);
}

/*
 *  ======== calloc ========
 */
void ATTRIBUTE *calloc(size_t nmemb, size_t size)
{
    size_t nbytes;
    void *retval;

    /* guard against divide by zero exception below */
    if (nmemb == 0) {
        errno = EINVAL;
        return (NULL);
    }

    nbytes = nmemb * size;

    /* return NULL if there's an overflow */
    if (nmemb && size != (nbytes / nmemb)) {
        errno = EOVERFLOW;
        return (NULL);
    }

    retval = malloc(nbytes);
    if (retval != NULL) {
        (void)memset(retval, (int)'\0', nbytes);
    }

    return (retval);
}

/*
 *  ======== realloc ========
 */
void ATTRIBUTE *realloc(void *ptr, size_t size)
{
    void *retval;
    Header *packet;
    size_t oldSize;

    if (ptr == NULL) {
        retval = malloc(size);
    }
    else if (size == 0) {
        errno = EINVAL;
        retval = NULL;
    }
    else {
        packet = (Header *)ptr - 1;
        retval = malloc(size);
        if (retval != NULL) {
            oldSize = packet->header.size - sizeof(Header);
            (void)memcpy(retval, ptr, (size < oldSize) ? size : oldSize);
            free(ptr);
        }
    }

    return (retval);
}

/*
 *  ======== free ========
 */
void ATTRIBUTE free(void *ptr)
{
    Header *packet;
    int diff = xPortGetFreeHeapSize();

    if (ptr != NULL) {
        packet = ((Header *)ptr) - 1;
        vPortFree(packet->header.actualBuf);
    }
    diff  = diff - xPortGetFreeHeapSize();

    Display_printf(display, 0, 0, "[%d] free (%p) %d", xPortGetFreeHeapSize(), ptr, diff);
}

/*
 *  ======== memalign ========
 */
void ATTRIBUTE *memalign(size_t boundary, size_t size)
{
    Header *packet;
    void   *tmp;

    /* return NULL if size is 0, or alignment is not a power-of-2 */
    if (size == 0 || (boundary & (boundary - 1))) {
        errno = EINVAL;
        return (NULL);
    }

    if (boundary < sizeof(Header)) {
        boundary = sizeof(Header);
    }

    /*
     * Allocate 'align + size + sizeof(Header)' so that we have room for
     * the 'packet' and can return an aligned buffer.
     */
    tmp = pvPortMalloc(boundary + size + sizeof(Header));

    if (tmp == NULL) {
        errno = ENOMEM;
        return (NULL);
    }

    if ((unsigned int)tmp & (boundary - 1)) {
        /* tmp is not already aligned */
        packet = (Header *)(((unsigned int)tmp + boundary) & ~(boundary - 1)) - 1;
        if (packet < (Header *)tmp) {
            /* don't have room for Header before aligned address */
            packet = (Header *)((unsigned int)packet + boundary);
        }
    }
    else {
        /* tmp is already aligned to boundary (by chance) */
        packet = ((Header *)(((unsigned int)tmp + boundary))) - 1;
    }

    packet->header.actualBuf = tmp;
    packet->header.size = size + sizeof(Header);

    return (packet + 1);
}

/*
 *  ======== aligned_alloc ========
 */
void ATTRIBUTE *aligned_alloc(size_t alignment, size_t size)
{
    /* use existing implementation to allocate buffer */
    return (memalign(alignment, size));
}

#if defined(__GNUC__) && !defined(__ti__)

/*
 *  ======== _malloc_r ========
 */
void ATTRIBUTE *_malloc_r(struct _reent *rptr, size_t size)
{
    return malloc(size);
}

/*
 *  ======== _calloc_r ========
 */
void ATTRIBUTE *_calloc_r(struct _reent *rptr, size_t nmemb, size_t size)
{
    return calloc(nmemb, size);
}

/*
 *  ======== _free_r ========
 */
void ATTRIBUTE _free_r(struct _reent *rptr, void *ptr)
{
    free(ptr);
}

/*
 *  ======== _realloc_r ========
 */
void ATTRIBUTE *_realloc_r(struct _reent *rptr, void *ptr, size_t size)
{
    return realloc(ptr, size);
}

#endif

we are seeing that simplelink's http and few other API's directly call pvPortMalloc and vPortFree instead of malloc /sl_malloc and free/sl_free, not sure how instrumenting malloc and free will help printing extra information?

where in the http library can you see a direct call to pvPortMalloc? 

Learnt from our team that, instead of providing wrappers to malloc and free, our engineer has replaced all the calls with pvportmalloc and vportfree, we will replace the modified files with actual ones, then will update the results.

Have you done more updates to the HTTPClient lib?

The other updated done was to replace all the sem_init and other semaphores calls with direct freertos calls, now we are porting our code base so that any of the ti files are not touched except the required porting files.

ok. I'll wait for the update.