Address several ESP32 network driver issues

[UncleGrumpy]

These changes close the following issues for the ESP32 network driver:

• [Unexpected behavior in esp32 network driver network:stop/0 #643] When network:stop/0 is used the driver is now completely stopped and all resources are freed.
• [Handle network_driver: Unhandled wifi event: 21. #1100] Adds a configurable event handler for STA beacon timeouts (Event: 21).
• Fixes several possible cases of double free() when using network:start/1.

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

[bettio]

network_driver_stop will not be called when calling network:stop/0. It gets called during tests (when GlobalContext is destoryed and recreated).
So this change sounds correct, but it doesn't do what you are saying.

[UncleGrumpy]

Thank you for the explanation of the REGISTER_PORT_DRIVER destroy_cb in the Telegram chat, this did not work as advertised! I was also several missing steps, so even when the VM exited and the destroy_cb would execute this code, netif and IP callbacks would still be triggered.

These problems have all been fixed, and the driver shuts down and exits cleanly. There is still one ESP-IDF message (in some builds, depending on the IDF version) in the console logs that is being reported as an error:

E (8246) wifi:NAN WiFi stop

I looked into this and it is actually a known ESP-IDF issue, that will be changed to an information message in a future update. See espressif/esp-idf#12473.

[bettio]

I'm not sure how this causes a crash. Can you elaborate further?

[UncleGrumpy]

This pointer has already been free'd between lines 703 and 715. Freeing it again will cause a double-free crash, this is an error condition already - but we want the user to get a nice Erlang stack trace (or catch the error), rather than a hard crash of the VM.

[bettio]

I tried to look at the sources, but I'm not understanding what you mean here. Can you elaborate further?

[pguyot]

@bettio I understand the crash could also be avoided if sta_wifi_config was set to NULL after line 716 (old) 736 (new). If sta_wifi_config was not NULL, it is freed anyway in the block above.

[UncleGrumpy]

Here is from release-0.6:
https://github.com/atomvm/AtomVM/blob/eb406a82e15c53d77304d30ceabcfc67a2d5e593/src/platforms/esp32/components/avm_builtins/network_driver.c#L724C1-L738C6

    if (!IS_NULL_PTR(sta_wifi_config)) {
        if ((err = esp_wifi_set_config(ESP_IF_WIFI_STA, sta_wifi_config)) != ESP_OK) {
            ESP_LOGE(TAG, "Error setting STA mode config %d", err);
            free(sta_wifi_config);
            if (!IS_NULL_PTR(ap_wifi_config)) {
                free(ap_wifi_config);
            }
            term error = port_create_error_tuple(ctx, term_from_int(err));
            port_send_reply(ctx, pid, ref, error);
            return;
        } else {
            ESP_LOGI(TAG, "STA mode configured");
            free(sta_wifi_config);
        }
    }

Either path through this code will free(sta_wifi_config). Freeing again below will cause a hard fault.

[UncleGrumpy]

@bettio I understand the crash could also be avoided if sta_wifi_config was set to NULL after line 716 (old) 736 (new). If sta_wifi_config was not NULL, it is freed anyway in the block above.

This might be an easier way to avoid the bug, but I still am not a fan of freeing the same variable multiple times, it always makes me have to stop an re-examine the code the next time I’m working on a feature, because it looks suspicious.

[petermm]

This is great!

Can anecdotally confirm this resolves esp32s2 OOM'ing with the forthcoming wifi test CI (oom only on esp-idf v5.1.4, on the test after wifi).. and adding network:stop() to the code resolved it. (3 failed runs OOM, 3 success runs with network:stop())

Have tested full Sim CI suite - all is green, the systems with wifi exercised network:stop().

[bettio]

Can anecdotally confirm this resolves esp32s2 OOM'ing with the forthcoming wifi test CI (oom only on esp-idf v5.1.4, on the test after wifi).. and adding network:stop() to the code resolved it. (3 failed runs OOM, 3 success runs with network:stop())

That is awesome, however I need a complete understanding about how this fix is working during the review...