Stack overflow due to recursive callback call

[tarzan115]

Hello @monstrenyatko Thank you for your library. I used your library in ESP8266 but when I got multiple messages in once, I got error stack buffer overflows. now I want to move ArrayBuffer to Heap to fix that error. or do you have any idea for that issue?

when I got any message I will feedback to another topic over MQTT. but when I got too many messages in once, my device can't feedback instant for each message. it will feedback after received done. Ex: Sender A sends 3 messages in once. My device got all messages after that my device send feedback. but when A sends more than 3 messages my device can't handle it and got error stack buffer overflows.

Can you help me with this error. thank you so much and have a good day.

[monstrenyatko]

Hi @tarzan115

If you create the ArrayBuffer like in example It is already on Heap:

MqttClient::Buffer *mqttSendBuffer = new MqttClient::ArrayBuffer<128>();
MqttClient::Buffer *mqttRecvBuffer = new MqttClient::ArrayBuffer<128>();

The client implementation is synchronous. You should not be able to receive more than one message simultaneously. If the sender sends 3 messages you will receive all 3 one by one. The mqttRecvBuffer is used for message reception. When the message received and assigned message handler performed all your actions the mqttRecvBuffer is reused to receive next message while you calling the yield method.

By design, you should not perform operations demanding to receive messages like publish (QOS 1 or 2) or subscribe/unsubscribe etc...because:

• the new incoming message will rewrite the content of mqttRecvBuffer => OK if you don't need the content (payload already copied to another buffer)
• another subscribed message could be received that is very dangerous because it will call another message handler into the context of the current handler => you might get stack overflow issue

Keep your message handler implementation simple => copy payload and set some flag or state => perform subscribe/unsubscribe or publish into the main loop if the flag is set. And don't forget to call yield regularly.

Always keep in mind that the microcontroller resources are limited. Stack and Heap are so small. We don't have threads to call the message handler in the different context.

PS: would be great to always see the code example to be able to reproduce your problem

[tarzan115]

Thank you so much. I really appreciate your help. I will look back and find another way.

thank you and have a good day.

[tarzan115]

here is my example code:

code to receive the message

static void
callback(MqttClient::MessageData& md)
{
  delay(0);
  const MqttClient::Message& msg = md.message;
  char payload[msg.payloadLen + 1];
  memcpy(payload, msg.payload, msg.payloadLen);
  payload[msg.payloadLen] = '\0';

  processJsonFromServer((char*)payload, msg.payloadLen + 1);
  return;
}

code to feedback

void
publishMQTT(const char* _Message, const bool _retained)
{
  delay(0);
  MqttClient::Message message;

  message.qos        = MqttClient::QOS1;
  message.retained   = _retained;
  message.dup        = false;
  message.payload    = (void *)_Message;
  message.payloadLen = strlen(_Message) + 1;

  mqtt->publish(MQTT_TOPIC_SEND, message);
}

here is my message from server

{
  "METHOD":"SET",
  "BTN":2,
  "STT":1,
  "TIME":1499912264,
  "MID":"-1befcd10:15d36a32b0a:-7fbb"
}

message feedback to server

{
  "METHOD":"BACK",
  "MID":"-1befcd10:15d36a32b0a:-7fbb",
  "TIME":1499912489,
  "ID":"16304264",
  "BTN":2,
  "STT":1
}

my process flow: Server send messages A, B and C to Client at the same time, Client processJsonFromServer and feedback message A'. but when I call function publishMQTT to feedback message A', buffer receive still have message B and C from Server so message A' will be store in buffer send. Client continue process with message B and go on.... Client still OK with 3 messages, but when have other message from Server in that process, Client will be die.

I set ArrayBuffer is 2000 for both SendBuffer and RecvBuffer.

do you have any idea to resolve that problem?

thank you for your time 😃

[monstrenyatko]

Hi @tarzan115

The receive buffer contains exactly one message because the client reads and processes the messages one by one. It definitely can't contain more than one message. Each new message overwrites the previous into the buffer. The receive buffer must be big enough to keep exactly one message. Do not waste the memory.

If you call the publishMQTT from processJsonFromServer The problem definitely with this function :

static void
callback(MqttClient::MessageData& md)
{
...
  char payload[msg.payloadLen + 1]; // !!! buffer on STACK !!!
...
  processJsonFromServer((char*)payload, msg.payloadLen + 1);
...
}

You allocate the temporary payload buffer on the stack. That is OK when buffer is not very big and the call is not recursive! publishMQTT uses QoS1. The publish with QoS1 requires calling of waitFor(PUBACK). The waitFor implementation might receive any message. In your case, you getting next message from the server while publish still waiting for the PUBACK. The problem that callback triggered by this next message from the server will be executed from the context of the publish and from the context of previous callback. Remember you have temporary payload buffer allocated on the stack? This next call allocates one more while previous one is still alive.

Let me make a chain of function calls in case of 3 messages sent by the server simultaneously: msg1 -> msg2 -> msg3 -> ESP8266 TCP buffer -> .... -> MqttClient::yield -> recvPacket(msg1) -> callback -> publishMQTT -> publish -> waitFor(PUBACK) -> recvPacket(msg2) -> callback -> publishMQTT -> publish -> waitFor(PUBACK) -> recvPacket(msg3) -> callback -> publishMQTT -> publish -> waitFor(PUBACK) -> ...

You definitely will get the stack overflow with this chain. I can propose you two solutions:

1. [SHORT term] publish with QoS0 that doesn't require the call of waitFor(PUBACK)
2. [LONG term] make the callback simpler => store received messages to some buffer on heap and process/publish from the main loop. Do not call anything like publish (QoS 1 or 2) or subscribe/unsubscribe from callback.

[tarzan115]

Hi @monstrenyatko thank you for detail about MQTT process. I understood. I will figure out the way to fix that issue.

thank you very much.

[monstrenyatko]

@tarzan115 No problem. Please let me know if you need more details.

[tarzan115]

@monstrenyatko sure, thank you 😃