Please explain esp_wifi_clear_ap_list() (IDFGH-9313)

[kriegste]

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

Hi,

in IDF 4.4.4 there is this new function called esp_wifi_clear_ap_list() ("Clear AP list found in last scan."). The description says: "When the obtained ap list fails,bss info must be cleared,otherwise it may cause memory leakage."

I do esp_wifi_scan_start() and in the event callback I use esp_wifi_scan_get_ap_num() and esp_wifi_scan_get_ap_records() to get the list of the scanned access points.
Exactly when do I need to use esp_wifi_clear_ap_list()? When the list fails? What does that mean? When esp_wifi_scan_get_ap_records() fails? There is no example.

[shentu]

You can try searching this function on github.
I found a example at https://github.com/AndreaGreco/GateOpener/blob/f76f98831d987b1d3645717008bd8984198a3810/main/wifi_config_ap.c#L90

[AxelLin]

You can try searching this function on github. I found a example at https://github.com/AndreaGreco/GateOpener/blob/f76f98831d987b1d3645717008bd8984198a3810/main/wifi_config_ap.c#L90

Above code you mentioned just calls esp_wifi_scan_get_ap_records() for each esp_wifi_scan_get_ap_records(), no matter it's success or fail. Are you sure this is correct? If so, then there must be a memory leak in older esp-idf versions.

From the wording of the comment: @brief Clear AP list found in last scan
I would think this should be called only when esp_wifi_scan_get_ap_records() success.
But the comments "@attention When the obtained ap list fails,bss info must be cleared,otherwise it may cause memory leakage."
seems imply this needs to be called when esp_wifi_scan_get_ap_records() fails.
Confused!

[kriegste]

If it needs to be called when esp_wifi_scan_get_ap_records fails, then I wonder why this is not automatically done inside esp_wifi_scan_get_ap_records (at the end before returning with an error) or inside esp_wifi_scan_start (at the start).

My confusion is in the sentence "when the [...] list fails". How can a list fail!?

[AxelLin]

@freakyxue

Could you help to explain the usage of esp_wifi_clear_ap_list() ?

[AxelLin]

@freakyxue @igrr
It's so difficult to get a response no matter on the github or forum https://www.esp32.com/viewtopic.php?f=13&t=32096

[AxelLin]

@freakyxue

Could you help to explain the usage of esp_wifi_clear_ap_list() ?

@freakyxue
Any update?
BTW, there is no caller using this API in esp-idf, I doubt if it really fixes anything?

[xueyunfei998]

hi @AxelLin

I'm sorry to reply until now.

When the memory is insufficient, perform wifi scan.， Failed to get wifi list.
At this time, it will cause memory leakage. At this time, need to call this interface to release the wifi list memory.

[AxelLin]

When the memory is insufficient, perform wifi scan.， Failed to get wifi list. At this time, it will cause memory leakage. At this time, need to call this interface to release the wifi list memory.

As I said this API is not used at all in current esp-idf.
Had better update the example codes to handle this possible memory leak.
BTW, the code in wpa_supplicant also does wifi-scan internally which does not use this API. This implies possible memory leak.

[AxelLin]

When the memory is insufficient, perform wifi scan.， Failed to get wifi list. At this time, it will cause memory leakage. At this time, need to call this interface to release the wifi list memory.

@xueyunfei998
Why this is not automatically done inside esp_wifi_scan_get_ap_records (at the end before returning with an error) or inside esp_wifi_scan_start (at the start).
Why you need to explicitly add this API to free memory?

[xueyunfei998]

scan_done_handler(uint16_t *sta_num, wifi_ap_record_t **ap_list_buffer_p)
{
char ssid[33];
uint32_t phy_mode=0;
uint16_t sta_number = 0;
uint8_t i;
wifi_ap_record_t *ap_list_buffer;
wifi_ap_record_t *ap_list;

esp_wifi_scan_get_ap_num(&sta_number);
*ap_list_buffer_p = ssc_os_malloc(sta_number * sizeof(wifi_ap_record_t));
ap_list_buffer = *ap_list_buffer_p;

if (sta_number != 0 && ap_list_buffer == NULL){
    esp_wifi_clear_ap_list();  
    return;
}

Give an example from above.

After failing to apply for memory in the callback function of scan done, need to call this interface to make clear the wifi list. If it is not clear, it will cause memory leakage. @AxelLin

[AxelLin]

scan_done_handler(uint16_t *sta_num, wifi_ap_record_t **ap_list_buffer_p) { char ssid[33]; uint32_t phy_mode=0; uint16_t sta_number = 0; uint8_t i; wifi_ap_record_t *ap_list_buffer; wifi_ap_record_t *ap_list;

@xueyunfei998

I don't get your point.
I don't know this API in your code:
scan_done_handler(uint16_t *sta_num, wifi_ap_record_t **ap_list_buffer_p)

The scan_done_handler in esp-idf has below prototype:
https://github.com/espressif/esp-idf/blob/master/examples/wifi/iperf/main/cmd_wifi.c#L66-L83
Do you mean it needs to call esp_wifi_clear_ap_list() if https://github.com/espressif/esp-idf/blob/master/examples/wifi/iperf/main/cmd_wifi.c#L80 ?

[xueyunfei998]

The scan_done_handler in esp-idf has below prototype: https://github.com/espressif/esp-idf/blob/master/examples/wifi/iperf/main/cmd_wifi.c#L66-L83 Do you mean it needs to call esp_wifi_clear_ap_list() if https://github.com/espressif/esp-idf/blob/master/examples/wifi/iperf/main/cmd_wifi.c#L80 ?

Yes, that's what I mean.

[AxelLin]

The scan_done_handler in esp-idf has below prototype: https://github.com/espressif/esp-idf/blob/master/examples/wifi/iperf/main/cmd_wifi.c#L66-L83 Do you mean it needs to call esp_wifi_clear_ap_list() if https://github.com/espressif/esp-idf/blob/master/examples/wifi/iperf/main/cmd_wifi.c#L80 ?

Yes, that's what I mean.

static void scan_done_handler() is a void function.
i.e. it does not return error!
Why a malloc failure in scan_done_handler() has impact on memory leak?
And it does not need to call esp_wifi_clear_ap_list() in success code path?
This is a very strange logic that I don't understand!

[xueyunfei998]

Because the wifi list in wifi lib is also dynamically allocated, the dynamically allocated memory in wifi lib will be released after successfully getting the wifi list in scan done. If the Wifi List in scan done fails, the dynamically allocated memory in wifi lib will not be released, resulting in memory leakage. @AxelLin

[AxelLin]

@xueyunfei998

I think what you want to say is:
esp_wifi_scan_get_ap_records() must be called in scan done handler, otherwise needs to call esp_wifi_clear_ap_list().
Is this understanding correct?
Is it necessary to call esp_wifi_clear_ap_list() if esp_wifi_scan_get_ap_records() fails?

[AxelLin]

@xueyunfei998

In below testing code (modify from examples/wifi/scan),
I didn't call esp_wifi_scan_get_ap_records() or esp_wifi_clear_ap_list() on purpose.
(I only call esp_wifi_scan_get_ap_num() to make sure the ap_count is not 0)
But I didn't find memory leakage. (Is there anything wrong in my testing code?)

while (1) {
esp_wifi_scan_start(NULL, true);
//ESP_ERROR_CHECK(esp_wifi_scan_get_ap_records(&number, ap_info)); // test memory leakage
ESP_ERROR_CHECK(esp_wifi_scan_get_ap_num(&ap_count));
ESP_LOGI(TAG, "Total APs scanned = %u", ap_count);
ESP_LOGI(TAG, "free_mem = %u", esp_get_free_heap_size());
vTaskDelay(5000 / portTICK_PERIOD_MS);
}

[kriegste]

I believe that "memory leak" is not the right description here. The found ap records occupy memory, but the IDF handles allocation and freeing correctly.
The problem that is solved with esp_wifi_clear_ap_list() is as follows: In low memory condition and when after esp_wifi_scan_start() the found ap records have eaten up another bite of memory, we at some point want to call esp_wifi_scan_get_ap_records() to retrieve those ap records. We first call malloc()- but it fails, we are out of memory. And we cannot call esp_wifi_scan_get_ap_records().
So the ap records stay in memory. To get back at least the memory allocated by them, we can call esp_wifi_clear_ap_list().

[xueyunfei998]

hi @kriegste

Your understanding is correct, but if the ssid memory in wifi lib is not released in time, there is no other opportunity to release this memory, which will lead to memory leakage.

[AxelLin]

hi @kriegste

Your understanding is correct, but if the ssid memory in wifi lib is not released in time, there is no other opportunity to release this memory, which will lead to memory leakage.

@xueyunfei998

But that does not explain why there is no memory leakage found in #10693 (comment) .
I didn't call esp_wifi_scan_get_ap_records() nor esp_wifi_clear_ap_list() on purpose.

[xueyunfei998]

hi @AxelLin

The following steps were followed to find the memory leak before

1 wifi scan
2 fail to get wifi list mentioned above
3 wifi stop
4 wifi deinit
5 wifi init (find memory leak)

Later, this bug was fixed,esp_wifi_clear_ap_list() was added, and bss info was cleared when wifi stop was performed.

You can view this commit in wifi lib (845af059ecbb64cb175df20cd6e61d956fdf2a66)

But that does not explain why there is no memory leakage found in #10693 (comment) . I didn't call esp_wifi_scan_get_ap_records() nor esp_wifi_clear_ap_list() on purpose.

You didn't find a memory leak because bss information will be re-released every time re-scan.