Closing HTTPS sockets returns ESP_FAIL - due to two lwip_close calls (IDFGH-4321)

[ghost]

Environment

• IDF version:
  v4.3-dev-1721-g9bcf8ad19
• Compiler version:
  xtensa-esp32-elf-gcc (crosstool-NG esp-2020r3) 8.4.0

Problem Description

A call to esp_http_client_close - after opening a connection to a HTTPS server - results in ESP_FAIL & errno being set to 9, which is Bad file number.
I've tracked the issue to lwip_close being indirectly called twice in

esp-idf/components/esp-tls/esp_tls.c

Lines 103 to 116 in 178b122

	int esp_tls_conn_destroy(esp_tls_t *tls)
	{
	if (tls != NULL) {
	int ret = 0;
	_esp_tls_conn_delete(tls);
	if (tls->sockfd >= 0) {
	ret = close(tls->sockfd);
	}
	esp_tls_internal_event_tracker_destroy(tls->error_handle);
	free(tls);
	return ret;
	}
	return -1; // invalid argument
	}

Once through

esp-idf/components/esp-tls/esp_tls.c

Line 107 in 178b122

_esp_tls_conn_delete(tls);

and again directly

esp-idf/components/esp-tls/esp_tls.c

Line 109 in 178b122

ret = close(tls->sockfd);

What is the correct course of action here? I'd suggest completely removing the call to close in esp_tls_conn_destroy (L109) as _esp_tls_conn_delete already calls the same function with the same parameter. Or, if it's there for the offchance that tls->is_tls is false, I'd change the if statement (L108) to if (tls->sockfd >= 0 && !tls->is_tls).

Debug Logs

// call to esp_http_client_close
D (36796) TRANS_SSL: [ssl_close(0x3ffca0a8)]
D (36806) esp-tls: [esp_tls_conn_destroy(0x3ffcb510)]

// call to _esp_tls_conn_delete(tls)
D (36806) esp-tls-mbedtls: [esp_mbedtls_conn_delete(0x3ffcb510)]:
sockets.c: [lwip_close(54)]
sockets.c: [lwip_close]: socket=0x3ffb4a54

D (36826) esp-tls: [esp_tls_conn_destroy] errno=0
D (36826) esp-tls: [esp_tls_conn_destroy] tls->sockfd=54

// call to close(tls->sockfd)
sockets.c: [lwip_close(54)]
sockets.c: [lwip_close]: socket=0x0

D (36846) esp-tls: [esp_tls_conn_destroy] errno=9
D (36846) esp-tls: [esp_tls_conn_destroy] ret=-1

[Alvin1Zhang]

Thanks for reporting, we will look into.

[AdityaHPatwardhan]

@istokm Thank you for reporting the issue and also suggesting the fix, the fix has already been merged internally. Should be updated on github in a couple of days.