Heap leak on tcp socket closing (IDFGH-8173)

[ifedotov1984]

Answers checklist.

• I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
• I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
• I have searched the issue tracker for a similar issue and not found a similar issue.

General issue report

When frequently open and close tcp sockets, anytime there are heap leakage by sizeof(struct tcp_pcb).
This is because the socket remains in the state TIME-WAIT.

Must be: When allocating a new socket, if the value is exceeded CONFIG_LWIP_MAX_ACTIVE_TCP (MEMP_NUM_TCP_PCB), the oldest socket in the state TIME-WAIT must be released.
Code from lwip:

tcp_alloc(u8_t prio, struct tcp_pcb_listen *tpcb)
{
  struct tcp_pcb *pcb;

  LWIP_ASSERT_CORE_LOCKED();

  pcb = (struct tcp_pcb *)memp_malloc(MEMP_TCP_PCB);
  if (pcb == NULL) {

    /* Try to send FIN for all pcbs stuck in TF_CLOSEPEND first */
    tcp_handle_closepend();

    /* Try killing oldest connection in TIME-WAIT. */
    LWIP_DEBUGF(TCP_DEBUG, ("tcp_alloc: killing off oldest TIME-WAIT connection\n"));
    tcp_kill_timewait();
    /* Try to allocate a tcp_pcb again. */
    pcb = (struct tcp_pcb *)memp_malloc(MEMP_TCP_PCB);
    if (pcb == NULL) {
      /* Try killing oldest connection in LAST-ACK (these wouldn't go to TIME-WAIT). */
      LWIP_DEBUGF(TCP_DEBUG, ("tcp_alloc: killing off oldest LAST-ACK connection\n"));
      tcp_kill_state(LAST_ACK);
      /* Try to allocate a tcp_pcb again. */
      pcb = (struct tcp_pcb *)memp_malloc(MEMP_TCP_PCB);
      if (pcb == NULL) {
        /* Try killing oldest connection in CLOSING. */
        LWIP_DEBUGF(TCP_DEBUG, ("tcp_alloc: killing off oldest CLOSING connection\n"));
        tcp_kill_state(CLOSING);
        /* Try to allocate a tcp_pcb again. */
        pcb = (struct tcp_pcb *)memp_malloc(MEMP_TCP_PCB);
        if (pcb == NULL) {
          /* Try killing oldest active connection with lower priority than the new one. */
          LWIP_DEBUGF(TCP_DEBUG, ("tcp_alloc: killing oldest connection with prio lower than %d\n", prio));
          tcp_kill_prio(prio);
          /* Try to allocate a tcp_pcb again. */
          pcb = (struct tcp_pcb *)memp_malloc(MEMP_TCP_PCB);
          if (pcb != NULL) {
            /* adjust err stats: memp_malloc failed multiple times before */
            MEMP_STATS_DEC(err, MEMP_TCP_PCB);
          }

What really: if MEMP_MEM_MALLOC=1 (by default), the value of CONFIG_LWIP_MAX_ACTIVE_TCP (MEMP_NUM_TCP_PCB) does not matter (not used in memp functions), and new sockets will be placed any time when calling tcp_alloc until the heap is zeroed out. And tcp_kill_timewait(); will call only when all ESP heap will be close to 0 (then memp_malloc(MEMP_TCP_PCB) will return NULL on allocation fails)
Setting MEMP_MEM_MALLOC=0 solves this issue, but there appears runtime issue with MEMP_NUM_SYS_TIMEOUT value (increasing this value solves this problem).

[shelacek]

There is a pull request targeting the same issue: espressif/esp-lwip#63.

[projectgus]

Just wanted to comment that we've also had reports of problems from too many sockets in TIME-WAIT, which this bug enables. If connections are short-lived and rapid then you can easily end up with hundreds of sockets in TIME-WAIT state, rather than limited to CONFIG_LWIP_MAX_ACTIVE_TCP (MEMP_NUM_TCP_PCB)

[david-cermak]

Hi Angus,
Thank you for bringing this up again, and I apologize for the delay in addressing it.

I can confirm that the current lwIP doesn't properly limit the number of active PCBs. Here's a preliminary patch that should resolve this, we'll fix this soon. I'm sorry the inconvenience.
bugfix-lwip-fix-tcp-pcb-allocation-max-num-limition.txt

[projectgus]

Hi @david-cermak, nice to hear from you again!

Appreciate the update and the patch, I know you folks are always busy.

For MicroPython's specific purposes we have our own patch now (PR is linked above), which basically uses a linker wrap to do the same thing as your patch - so we're good until this gets fixed upstream. 👍