AsyncTCP on ESP32 and Odd Heap/Socket Issues w/SOFTAP

Question

I'm struggling with an issue where an ESP32 is running as a AP with AsyncTCP connecting multiple ESP32 clients. The AP receives some JSON data and replies with some JSON data. Without the handleData() function, the code runs 100% fine with no issues. Heap is static when no clients connect and issues only occur when clients start connecting.

Can anyone see anything with my code that could be causing heap corruption or other memory weirdness?

static void handleData(void* arg, AsyncClient* client, void *data, size_t len) {
int i = 0, j = 0;
char clientData[CLIENT_DATA_MAX];
char packetData[len];
char *packetBuf;

   packetBuf = (char *)data;
   clientData[0] = '\0';

   for (i=0;i <= len;i++) {
      packetData[j] = packetBuf[i]; //packetBuf[i];

      if ((packetData[j] == '\n') || (i == len)) {
         packetData[j] = '\0';
         if ((j > 0) && (packetData[0] != '\n') && (packetData[0] != '\r')) {
            // See sensorData() below...
            parseData.function(packetData, clientData);
            if (clientData != NULL) {
               // TCP reply to client
               if (client->space() > 32 && client->canSend()) {
                 client->write(clientData);
               }
            }
         }
         j = 0;
      } else
         j++;
   }
}

void sensorData(void *data, void *retData) {
StaticJsonDocument<CLIENT_DATA_MAX> fields;
StaticJsonDocument<CLIENT_DATA_MAX> output;
char sensor[15] = "\0";
char MAC[18] = "\0";
char value[20] = "\0";
bool sendOK = false;

   memcpy((char *)retData, "\0", 1);
   DeserializationError error = deserializeJson(fields, (char *)data, CLIENT_DATA_MAX);
   if (error) {
      DEBUG_PRINTLN(F("deserializeJson() failed"));
      return;
   }

   if (fields["type"])
      strcpy(sensor, fields["type"]);

   switch (sensor[0]) {
      case 'C': 
         if (fields["value"])
            strcpy(value, fields["value"]);
         sendOK = true;
         break;
      case 'T': //DEBUG_PRINT(F("Temp "));
         setExtTempSensor(fields["value"]);
         sendOK = true;
         break;
      case 'N': 
         output["IT"] = intTempC; //Internal temp
         output["B1"] = battLevels[0];
         serializeJson(output, (char *)retData, CLIENT_DATA_MAX-1);
         break;
   } 
   if (sendOK) {
      output["Resp"] = "Ok";
      serializeJson(output, (char *)retData, CLIENT_DATA_MAX-1);
   }
   strcat((char *)retData, "\n");
}

static void handleNewClient(void* arg, AsyncClient* client) {
   client->setRxTimeout(1000);
   client->setAckTimeout(500);
   client->onData(&handleData, NULL);
   client->onError(&handleError, NULL);
   client->onDisconnect(&handleDisconnect, NULL);
   client->onTimeout(&handleTimeOut, NULL);
}

void startServer() {
  server = new AsyncServer(WIFI_SERVER_PORT);
  server->onClient(&handleNewClient, &server)
}

score 1 · Answer 1 · answered Jun 25 '20 at 19:59

Using AsyncTCP on the ESP32 was having multiple issues. Heap issues, socket issues, assert issues, ACK timeouts, connection timeouts, etc. Swapping to AsyncUDP using the exact same code as shown above with romkey's changes, resolved all of my issues. (Just using romkey's fixes did not fix the errors I was having with AsyncTCP.) I don't believe the issue is with AsyncTCP but with ESP32 libraries.

score 0 · Answer 2 · answered Jun 22 '20 at 15:04

Either you should declare packetData to be of length len + 1 or your for loop should iterate until i < len. Because the index starts at 0, packetData[len] is actually byte len + 1, so you'll overwrite something random when you store something in packetData[len] if the array is only len chars long.That something random may be the pointer stored in packetBuf, which could easily cause heap corruption.

You should always use strncpy() and never strcpy(). Likewise use strncat() rather than strcat(). Don't depend on having done the math correctly or on sizes not changing as your code evolves. strncpy() and strncat() will guard against overflows. You'll need to pass a length into sensorData() to do that, but sensorData() shouldn't be making assumptions about the available length of retData.

Your test

    if (clientData != NULL) {

will never fail because clientData is the address of array and cannot change. I'm not sure what you're trying to test for here but this if will always succeed.

You can just write:

char sensor[15] = "";

you don't need to explicitly assign a string with a null byte in it.

And

memcpy((char *)retData, "\0", 1);

is equivalent to

((char *)retData)[0] = '\0';

What's the point of declaring retData to be void * in the arguments to sensorData()? Your code starts out with it being a char* before calling sensorData() and uses it as a char* inside sensorData(). void * is meant to be an escape hatch for passing around pointers without worrying about their type. You don't need that here and end up needing to extra casts back to char* because of it. Just declare the argument to be char* and don't worry about casting it again.

You didn't share the code that calls handleData() so there may well be issues outside of these functions.

I made the recommended changes and crashes are still occuring. However, wouldn't any corrupt memory that happens due to local variables cause issues with the stack and not the heap? I updated the original code to include the code that calls handleData(). — YachtLover, Jun 24 '20 at 09:41
You have pointers on the stack, like `packetBuf`, so if you corrupt the stack you may start accessing random parts of memory and damage the heap. — romkey, Jun 24 '20 at 12:52

AsyncTCP on ESP32 and Odd Heap/Socket Issues w/SOFTAP

2 Answers2