Crash when Debug port is Disabled - HTTPClient destructor issue?

[joysfera]

Basic Infos

• This issue complies with the issue POLICY doc.
• I have read the documentation at readthedocs and the issue is not addressed there.
• I have tested that the issue is present in current master branch (aka latest git).
• I have searched the issue tracker for a similar issue.
• If there is a stack dump, I have decoded it.
• I have filled out all fields below.

Platform

• Hardware: [ESP-12]
• Core Version: [just released 2.5.0]
• Development Env: [Arduino IDE 1.8.8]
• Operating System: [Ubuntu]

Settings in IDE

• Module: [Generic ESP8266 Module]
• Flash Mode: [dout]
• Flash Size: [4MB]
• lwip Variant: [v2 Lower Memory]
• Reset Method: [ck]
• Flash Frequency: [40Mhz]
• CPU Frequency: [80Mhz]
• Upload Using: [SERIAL]
• Upload Speed: [115200]

Problem Description

In Arduino IDE when Debug port is set to Disabled the ESP8266 reboots (most probably due to a crash) a short while after fetching JSON data via https, parsing it and displaying it on a LED matrix. However, if I set the Debug port to Serial then everything works properly. My application does not use any debugging stuff. It does not even use the Serial stuff, does not call Serial.begin(), nothing of that.

The Debug level is set to None all the time so there's no reason why Debug port setting should be significant or should affect the application in any way, IMHO.

I cannot debug this because when I enable debugging and thus set the Debug port to Serial the issue disappears :-)

EDIT: here's the minimal code:

#include <ESP8266WiFi.h>
#include <ESP8266HTTPClient.h>
#include <WiFiClientSecureBearSSL.h>

const int led = 5;

void setup(void)
{
  pinMode(led, OUTPUT);
  for(byte i=0; i<5; i++) {
    digitalWrite(led, HIGH);
    delay(150);
    digitalWrite(led, LOW);
    delay(200);
  }
  
  WiFi.mode(WIFI_STA);
  // Wait for connection
  while (WiFi.status() != WL_CONNECTED) {
    delay(250);
  }
}

void displayData()
{
  digitalWrite(led, HIGH);
  HTTPClient http;
  std::unique_ptr<BearSSL::WiFiClientSecure>client(new BearSSL::WiFiClientSecure);
  client->setInsecure();
  if (http.begin(*client, "https://jigsaw.w3.org/HTTP/connection.html")) {
    if (http.GET() == 200) {
      String payload = http.getString();
    }
    http.end();
  }
  digitalWrite(led, LOW);
}

void loop(void)
{
  static unsigned long mil = 0;
  if (millis() - mil > 5000) {   
    displayData();
    mil = millis();
  }
}

It's easy to reproduce that mere Debug port setting turns application crashing to not crashing and vice-versa.

It's obvious from the source code above that the LED should blink 5 times at start, then wait for up to 5 seconds, and then blink (when the HTTPS request is invoked) once every five seconds.
What you observe is that immediately after the longer HTTPS blink there's the boot-up fast 5 blinks. That indicates the crash&reboot.

EDIT2: you might notice that my MCVE is an almost verbatim copy of the "BasicHttpsClient" example that is provided with Arduino core. Interesting is that the stock BasicHttpsClient does not crash. What is different? Well, the only interesting difference is the order of HTTPClient and BearSL client instantiation. And that's actually it! If you switch the order of those two lines in the BasicHttpsClient example (so that HTTPClient is first, the std::unique_ptr is second) it will start crashing as well, this time with a stack trace:

[SETUP] WAIT 4...
[SETUP] WAIT 3...
[SETUP] WAIT 2...
[SETUP] WAIT 1...
[HTTPS] begin...
[HTTPS] GET...

Exception (9):
epc1=0x40206cb9 epc2=0x00000000 epc3=0x00000000 excvaddr=0x000001cb depc=0x00000000

>>>stack>>>

ctx: cont
sp: 3ffffd60 end: 3fffffc0 offset: 01a0
3fffff00:  00000001 3ffeecd8 3ffefd14 40202842  
3fffff10:  00000000 00000000 3ffefd14 3ffefa94  
3fffff20:  0000000f 0000000d 000001bb 40001388  
3fffff30:  3ffefe1c 0000001f 00000015 3ffefac4  
3fffff40:  0000000f 00000005 3ffefadc 0000000f  
3fffff50:  00000000 3ffefb14 0000001f 00000011  
3fffff60:  3ffefb3c 0000000f 00000000 00000000  
3fffff70:  00000000 00000000 ffffffff 3fffef00  
3fffff80:  00000000 00000000 00000000 00000000  
3fffff90:  00000000 00000000 3ffeecd8 40202761  
3fffffa0:  3fffdad0 00000000 3ffeed20 402082b4  
3fffffb0:  feefeffe feefeffe 3ffe8508 401009e9  
<<<stack<<<

 ets Jan  8 2013,rst cause:2, boot mode:(1,7)


 ets Jan  8 2013,rst cause:4, boot mode:(1,7)

wdt reset

[devyte]

@joysfera This comment is relevant:

/*
 * Since both begin() functions take a reference to client as a parameter, you need to 
 * ensure the client object lives the entire time of the HTTPClient
 */

HttpClient keeps a pointer to client, and tries to access it in the destructor. In your order of instantiation, client gets destroyed first. Then, when HttpClient gets destroyed, it tries to access client, but it no longer exists, so garbage => crash.

Closing due to user error.

[joysfera]

@devyte thanks. I've created a PR #5739 to save others the lost time due to wrong example code.

[yoursunny]

This seems to be bad API design rather than simply blaming on user error.
To avoid such mistakes, HTTPClient should take WifiClient& reference either in its constructor, or as a unique_ptr.

std::unique_ptr<BearSSL::WiFiClientSecure> client(new BearSSL::WiFiClientSecure);
HTTPClient http(*client);
http.begin(uri);
// unless user intentionally destructs ‘client’, it’s safe

HTTPClient http;
std::unique_ptr<BearSSL::WiFiClientSecure> client(new BearSSL::WiFiClientSecure);
http.begin(std::move(client), uri);
// ‘client’ is solely owned by the HTTPClient now

[d-a-v]

Reopening per https://gitter.im/esp8266/Arduino?at=60bfcfe502f6ee5b91e06200 and per closed unfinished #5739 .
@holgerlembke

[holgerlembke]

Ok, please help me to understand the problem. First, reuse. I look at the ReuseConnection-Example from ESP8266HTTPClient.

Where is the connection-resue? The loop() has "WiFiClient client;" so I assume the connection is created everytime.

A valid reuse-example would look like:

/**
   reuseConnection.ino

    Created on: 22.11.2015

*/


#include <Arduino.h>

#include <ESP8266WiFi.h>
#include <ESP8266WiFiMulti.h>

#include <ESP8266HTTPClient.h>

ESP8266WiFiMulti WiFiMulti;

HTTPClient http;
WiFiClient client;

void setup() {

  Serial.begin(115200);
  // Serial.setDebugOutput(true);

  Serial.println();
  Serial.println();
  Serial.println();

  for (uint8_t t = 4; t > 0; t--) {
    Serial.printf("[SETUP] WAIT %d...\n", t);
    Serial.flush();
    delay(1000);
  }

  WiFi.mode(WIFI_STA);
  WiFiMulti.addAP("SSID", "PASSWORD");

  // allow reuse (if server supports it)
  http.setReuse(true);
}

void loop() {
  // wait for WiFi connection
  if ((WiFiMulti.run() == WL_CONNECTED)) {

    if (client.connected()) {
      Serial.println("\n!!Connected!!!\n");
    }

    http.begin(client, "http://jigsaw.w3.org/HTTP/connection.html");
    //http.begin(client, "jigsaw.w3.org", 80, "/HTTP/connection.html");

    int httpCode = http.GET();
    if (httpCode > 0) {
      Serial.printf("[HTTP] GET... code: %d\n", httpCode);

      // file found at server
      if (httpCode == HTTP_CODE_OK) {
        http.writeToStream(&Serial);
      }
    } else {
      Serial.printf("[HTTP] GET... failed, error: %s\n", http.errorToString(httpCode).c_str());
    }

    http.end();
  }

  delay(5000);
}

Right?

[d-a-v]

Where is the connection-reused?

In the HTTP client class.
The first time connection is made HTTP/1.1, client connection is not closed, is kept open between loops by http instance.

The loop() has "WiFiClient client;" so I assume the connection is created everytime.

In your example it is not: it is declared globally. That's how it can be kept opened between loops.

[holgerlembke]

Really? In begin(client...) _client is assigned the newly created WiFiClient.

[d-a-v]

Really? In begin(client...) _client is assigned the newly created WiFiClient.

It is given, not created. What http client does it it under the hood might be different from what you think (now I understand your question in gitter).
http.begin(client, url) can be thought of "begin a new request, create the connection if needed, reuse it if possible".