From 1984f76eb8301d21a9d051ac55e7eca4d18c9f7b Mon Sep 17 00:00:00 2001 From: Winford Date: Tue, 30 Apr 2024 19:26:45 -0700 Subject: [PATCH 1/3] Fix several possible double free() in ESP32 network_driver.c Removes several possible uses of free() on memory that had been previously free'd. This would happen under specific error conditions in the `start_network` function in network_driver.c that `network:start/1` uses to configure and start the network. Signed-off-by: Winford --- CHANGELOG.md | 2 ++ .../esp32/components/avm_builtins/network_driver.c | 7 ++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4884eedb..ff4d071f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,8 @@ 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. ## [0.6.4] - 2024-08-18 diff --git a/src/platforms/esp32/components/avm_builtins/network_driver.c b/src/platforms/esp32/components/avm_builtins/network_driver.c index 6252e6196..78a33449e 100644 --- a/src/platforms/esp32/components/avm_builtins/network_driver.c +++ b/src/platforms/esp32/components/avm_builtins/network_driver.c @@ -744,7 +744,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 +752,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"); From ad3c796845aaa0114af55f65da3c9cfd5a9c942b Mon Sep 17 00:00:00 2001 From: Winford Date: Sun, 21 Apr 2024 15:54:40 -0700 Subject: [PATCH 2/3] Completly stop driver and free all resources with network:stop/0 on ESP32 Adds a destroy callback to the ESP32 network driver to completely stop the driver and free all network resources when network:stop/0 is used. Previosly the driver was not being stopped internally and resources were not freed when the gen_server was stopped, causing instability, and possible crashes when event callbacks were triggered, but there was no process alive to handle them. Closes #643 Signed-off-by: Winford --- CHANGELOG.md | 2 + libs/eavmlib/src/network.erl | 2 + .../components/avm_builtins/network_driver.c | 51 +++++++++++++++++-- 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff4d071f4..b79865094 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,8 @@ instead - 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/libs/eavmlib/src/network.erl b/libs/eavmlib/src/network.erl index 9943e12f2..cb4f3961f 100644 --- a/libs/eavmlib/src/network.erl +++ b/libs/eavmlib/src/network.erl @@ -386,6 +386,8 @@ handle_info(Msg, State) -> %% @hidden terminate(_Reason, _State) -> + Ref = make_ref(), + network_port ! {?SERVER, Ref, stop}, ok. %% diff --git a/src/platforms/esp32/components/avm_builtins/network_driver.c b/src/platforms/esp32/components/avm_builtins/network_driver.c index 78a33449e..93ba60bad 100644 --- a/src/platforms/esp32/components/avm_builtins/network_driver.c +++ b/src/platforms/esp32/components/avm_builtins/network_driver.c @@ -92,12 +92,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) }; @@ -779,12 +781,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); @@ -805,11 +846,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; @@ -842,6 +883,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); @@ -868,7 +913,7 @@ static NativeHandlerResult consume_mailbox(Context *ctx) mailbox_remove_message(&ctx->mailbox, &ctx->heap); - return NativeContinue; + return cmd_terminate ? NativeTerminate : NativeContinue; } // From ec7602c6c440eb1a799ae02bcee6d384b4f6347a Mon Sep 17 00:00:00 2001 From: Winford Date: Fri, 19 Apr 2024 19:02:41 -0700 Subject: [PATCH 3/3] Add ESP32 beacon timeout event handler Adds an event handler for `event 21` the `WIFI_EVENT_STA_BEACON_TIMEOUT` event and an option to add an Erlang callback handler for the event. The event will be logged with an info level message that includes a suggestion about the two most likely causes, poor rssi and network congestion. A callback config option `{beacon_timeout, fun()}` may be added to the `sta` config. Closes #1100 Signed-off-by: Winford --- CHANGELOG.md | 2 ++ doc/src/network-programming-guide.md | 1 + libs/eavmlib/src/network.erl | 9 +++++++++ .../components/avm_builtins/network_driver.c | 19 +++++++++++++++++++ 4 files changed, 31 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b79865094..8b85864c5 100644 --- 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 diff --git a/doc/src/network-programming-guide.md b/doc/src/network-programming-guide.md index 55912489f..c46346b46 100644 --- 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-">>`, where `` 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 index cb4f3961f..3f83ca82c 100644 --- a/libs/eavmlib/src/network.erl +++ b/libs/eavmlib/src/network.erl @@ -50,6 +50,7 @@ -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() :: @@ -57,6 +58,7 @@ | 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}; @@ -398,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 index 93ba60bad..46ee0dc48 100644 --- 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"); @@ -168,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"); @@ -304,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;