Merge pull request #1137 from UncleGrumpy/esp32_network_issues

Address several ESP32 network driver issues These changes close the following issues for the ESP32 network driver: * [#643] When network:stop/0 is used the driver is now completely stopped and all resources are freed. * [#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
atomvm · Oct 3, 2024 · 3c80b39 · 3c80b39
2 parents 87716e8 + ec7602c
commit 3c80b39
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -37,6 +37,8 @@ also non string parameters (e.g. `Enum.join([1, 2], ",")`
 - Support for directory listing using POSIX APIs: (`atomvm:posix_opendir/1`,
 `atomvm:posix_readdir/1`, `atomvm:posix_closedir/1`).
 - ESP32: add support for `esp_adc` ADC driver, with Erlang and Elixir examples
+- Add handler for ESP32 network driver STA mode `beacon_timeout` (event: 21), see issue
+[#1100](https://github.com/atomvm/AtomVM/issues/1100)
 
 ### Changed
 
@@ -55,6 +57,10 @@ instead
 - `unicode:characters_to_list`: fixed bogus out_of_memory error on some platforms such as ESP32
 - Fix crash in Elixir library when doing `inspect(:atom)`
 - General inspect() compliance with Elixir behavior (but there are still some minor differences)
+- Fix several uses of free on prevously released memory on ESP32, under certain error condition using
+`network:start/1`, that would lead to a hard crash of the VM.
+- Fix a bug in ESP32 network driver where the low level driver was not being stopped and resoureces were not freed
+when `network:stop/0` was used, see issue [#643](https://github.com/atomvm/AtomVM/issues/643)
 
 ## [0.6.4] - 2024-08-18
 

diff --git a/doc/src/network-programming-guide.md b/doc/src/network-programming-guide.md
@@ -46,6 +46,7 @@ Callback functions are optional, but are highly recommended for building robust
 In addition, the following optional parameters can be specified to configure the AP network (ESP32 only):
 
 * `{dhcp_hostname, string()|binary()}` The DHCP hostname as which the device should register (`<<"atomvm-<hexmac>">>`, where `<hexmac>` is the hexadecimal representation of the factory-assigned MAC address of the device).
+* `{beacon_timeout, fun(() -> term())}` A callback function which will be called when the device does not receive a beacon frame from the connected access point during the "inactive time" (6 second default, currently not configurable).
 
 The following example illustrates initialization of the WiFi network in STA mode.  The example program will configure the network to connect to a specified network.  Events that occur during the lifecycle of the network will trigger invocations of the specified callback functions.
 

diff --git a/libs/eavmlib/src/network.erl b/libs/eavmlib/src/network.erl
@@ -50,13 +50,15 @@
 
 -type dhcp_hostname_config() :: {dhcp_hostname, string() | binary()}.
 -type sta_connected_config() :: {connected, fun(() -> term())}.
+-type sta_beacon_timeout_config() :: {beacon_timeout, fun(() -> term())}.
 -type sta_disconnected_config() :: {disconnected, fun(() -> term())}.
 -type sta_got_ip_config() :: {got_ip, fun((ip_info()) -> term())}.
 -type sta_config_property() ::
     ssid_config()
     | psk_config()
     | dhcp_hostname_config()
     | sta_connected_config()
+    | sta_beacon_timeout_config()
     | sta_disconnected_config()
     | sta_got_ip_config().
 -type sta_config() :: {sta, [sta_config_property()]}.
@@ -357,6 +359,9 @@ handle_cast(_Msg, State) ->
 handle_info({Ref, sta_connected} = _Msg, #state{ref = Ref, config = Config} = State) ->
     maybe_sta_connected_callback(Config),
     {noreply, State};
+handle_info({Ref, sta_beacon_timeout} = _Msg, #state{ref = Ref, config = Config} = State) ->
+    maybe_sta_beacon_timeout_callback(Config),
+    {noreply, State};
 handle_info({Ref, sta_disconnected} = _Msg, #state{ref = Ref, config = Config} = State) ->
     maybe_sta_disconnected_callback(Config),
     {noreply, State};
@@ -386,6 +391,8 @@ handle_info(Msg, State) ->
 
 %% @hidden
 terminate(_Reason, _State) ->
+    Ref = make_ref(),
+    network_port ! {?SERVER, Ref, stop},
     ok.
 
 %%
@@ -396,6 +403,10 @@ terminate(_Reason, _State) ->
 maybe_sta_connected_callback(Config) ->
     maybe_callback0(connected, proplists:get_value(sta, Config)).
 
+%% @private
+maybe_sta_beacon_timeout_callback(Config) ->
+    maybe_callback0(beacon_timeout, proplists:get_value(sta, Config)).
+
 %% @private
 maybe_sta_disconnected_callback(Config) ->
     maybe_callback0(disconnected, proplists:get_value(sta, Config)).

diff --git a/src/platforms/esp32/components/avm_builtins/network_driver.c b/src/platforms/esp32/components/avm_builtins/network_driver.c
@@ -76,6 +76,7 @@ static const char *const ssid_atom = ATOM_STR("\x4", "ssid");
 static const char *const ssid_hidden_atom = ATOM_STR("\xB", "ssid_hidden");
 static const char *const sta_atom = ATOM_STR("\x3", "sta");
 static const char *const sta_connected_atom = ATOM_STR("\xD", "sta_connected");
+static const char *const sta_beacon_timeout_atom = ATOM_STR("\x12", "sta_beacon_timeout");
 static const char *const sta_disconnected_atom = ATOM_STR("\x10", "sta_disconnected");
 static const char *const sta_got_ip_atom = ATOM_STR("\xA", "sta_got_ip");
 
@@ -92,12 +93,14 @@ enum network_cmd
     NetworkInvalidCmd = 0,
     // TODO add support for scan, ifconfig
     NetworkStartCmd,
-    NetworkRssiCmd
+    NetworkRssiCmd,
+    NetworkStopCmd
 };
 
 static const AtomStringIntPair cmd_table[] = {
     { ATOM_STR("\x5", "start"), NetworkStartCmd },
     { ATOM_STR("\x4", "rssi"), NetworkRssiCmd },
+    { ATOM_STR("\x4", "stop"), NetworkStopCmd },
     SELECT_INT_DEFAULT(NetworkInvalidCmd)
 };
 
@@ -166,6 +169,18 @@ static void send_sta_connected(struct ClientData *data)
     END_WITH_STACK_HEAP(heap, data->global);
 }
 
+static void send_sta_beacon_timeout(struct ClientData *data)
+{
+    TRACE("Sending sta_beacon_timeout back to AtomVM\n");
+
+    // {Ref, sta_beacon_timeout}
+    BEGIN_WITH_STACK_HEAP(PORT_REPLY_SIZE, heap);
+    {
+        send_term(&heap, data, make_atom(data->global, sta_beacon_timeout_atom));
+    }
+    END_WITH_STACK_HEAP(heap, data->global);
+}
+
 static void send_sta_disconnected(struct ClientData *data)
 {
     TRACE("Sending sta_disconnected back to AtomVM\n");
@@ -302,6 +317,12 @@ static void event_handler(void *arg, esp_event_base_t event_base, int32_t event_
             }
 #endif
 
+            case WIFI_EVENT_STA_BEACON_TIMEOUT: {
+                ESP_LOGI(TAG, "WIFI_EVENT_STA_BEACON_TIMEOUT received. Maybe poor signal, or network congestion?");
+                send_sta_beacon_timeout(data);
+                break;
+            }
+
             default:
                 ESP_LOGI(TAG, "Unhandled wifi event: %" PRIi32 ".", event_id);
                 break;
@@ -744,7 +765,6 @@ static void start_network(Context *ctx, term pid, term ref, term config)
         if ((err = esp_wifi_set_config(ESP_IF_WIFI_AP, ap_wifi_config)) != ESP_OK) {
             ESP_LOGE(TAG, "Error setting AP mode config %d", err);
             free(ap_wifi_config);
-            free(sta_wifi_config);
             term error = port_create_error_tuple(ctx, term_from_int(err));
             port_send_reply(ctx, pid, ref, error);
             return;
@@ -753,12 +773,14 @@ static void start_network(Context *ctx, term pid, term ref, term config)
             free(ap_wifi_config);
         }
     }
+
+    //
+    // Start the configured interface(s)
+    //
     if ((err = esp_wifi_start()) != ESP_OK) {
         ESP_LOGE(TAG, "Error in esp_wifi_start %d", err);
         term error = port_create_error_tuple(ctx, term_from_int(err));
         port_send_reply(ctx, pid, ref, error);
-        free(ap_wifi_config);
-        free(sta_wifi_config);
         return;
     } else {
         ESP_LOGI(TAG, "WIFI started");
@@ -778,12 +800,51 @@ static void start_network(Context *ctx, term pid, term ref, term config)
     if (!IS_NULL_PTR(ap_wifi_config)) {
         set_dhcp_hostname(ap_wifi_interface, "AP", interop_kv_get_value(ap_config, dhcp_hostname_atom, ctx->global));
     }
+
     //
     // Done -- send an ok so the FSM can proceed
     //
     port_send_reply(ctx, pid, ref, OK_ATOM);
 }
 
+static void stop_network(Context *ctx)
+{
+    // Stop unregister event callbacks so they dont trigger during shutdown.
+    esp_event_handler_unregister(WIFI_EVENT, ESP_EVENT_ANY_ID, &event_handler);
+
+    esp_netif_t *sta_wifi_interface = esp_netif_get_handle_from_ifkey("WIFI_STA_DEF");
+    esp_netif_t *ap_wifi_interface = esp_netif_get_handle_from_ifkey("WIFI_AP_DEF");
+
+    // Disconnect STA if connected to access point
+    if ((sta_wifi_interface != NULL) && (esp_netif_is_netif_up(sta_wifi_interface))) {
+        esp_err_t err = esp_wifi_disconnect();
+        if (UNLIKELY(err == ESP_FAIL)) {
+            ESP_LOGE(TAG, "ESP FAIL error while disconnecting from AP, continuing network shutdown...");
+        }
+    }
+
+    // Stop and deinit the WiFi driver, these only return OK, or not init error (fine to ignore).
+    esp_wifi_stop();
+    esp_wifi_deinit();
+
+    // Stop sntp (ignore OK, or not configured error)
+    esp_sntp_stop();
+
+    // Delete network event loop
+    esp_err_t err = esp_event_loop_delete_default();
+    if (err != ESP_OK) {
+        ESP_LOGE(TAG, "Invalid state error while deleting event loop, continuing network shutdown...");
+    }
+
+    // Destroy existing netif interfaces
+    if (ap_wifi_interface != NULL) {
+        esp_netif_destroy_default_wifi(ap_wifi_interface);
+    }
+    if (sta_wifi_interface != NULL) {
+        esp_netif_destroy_default_wifi(sta_wifi_interface);
+    }
+}
+
 static void get_sta_rssi(Context *ctx, term pid, term ref)
 {
     size_t tuple_reply_size = PORT_REPLY_SIZE + TUPLE_SIZE(2);
@@ -804,11 +865,11 @@ static void get_sta_rssi(Context *ctx, term pid, term ref)
     port_ensure_available(ctx, tuple_reply_size);
     term reply = port_create_tuple2(ctx, make_atom(ctx->global, ATOM_STR("\x4", "rssi")), rssi);
     port_send_reply(ctx, pid, ref, reply);
-
 }
 
 static NativeHandlerResult consume_mailbox(Context *ctx)
 {
+    bool cmd_terminate = false;
     Message *message = mailbox_first(&ctx->mailbox);
     term msg = message->message;
 
@@ -841,6 +902,10 @@ static NativeHandlerResult consume_mailbox(Context *ctx)
             case NetworkRssiCmd:
                 get_sta_rssi(ctx, pid, ref);
                 break;
+            case NetworkStopCmd:
+                cmd_terminate = true;
+                stop_network(ctx);
+                break;
 
             default: {
                 ESP_LOGE(TAG, "Unrecognized command: %x", cmd);
@@ -867,7 +932,7 @@ static NativeHandlerResult consume_mailbox(Context *ctx)
 
     mailbox_remove_message(&ctx->mailbox, &ctx->heap);
 
-    return NativeContinue;
+    return cmd_terminate ? NativeTerminate : NativeContinue;
 }
 
 //