Merge pull request #1137 from david-cermak/fix/mdns_browse_sync_end_race

fix(mdns): Fix mdns browse SYNC vs. END race

Author: david-cermak

.github/workflows/mdns__build-target-test.yml

@@ -13,7 +13,7 @@ jobs:
     name: Build
     strategy:
       matrix:
-        idf_ver: ["latest", "release-v5.0", "release-v5.2", "release-v5.3"]
+        idf_ver: ["latest", "release-v5.5", "release-v6.0", "release-v6.1"]
         test: [ { app: example, path: "examples/query_advertise" }, { app: unit_test, path: "tests/unit_test" }, { app: test_app, path: "tests/test_apps" } ]
     runs-on: ubuntu-22.04
     container: espressif/idf:${{ matrix.idf_ver }}
@@ -24,7 +24,7 @@ jobs:
         shell: bash
         run: |
           . ${IDF_PATH}/export.sh
-          if [[ "${{ matrix.idf_ver }}" == "latest" ]]; then
+          if [[ "${{ matrix.idf_ver }}" == "latest" || "${{ matrix.idf_ver }}" == release-v6.* ]]; then
             export EXPECTED_WARNING="warning: unknown kconfig symbol 'EXAMPLE_ETH_PHY_IP101'"
           else
             export EXPECTED_WARNING="warning: unknown kconfig symbol 'EXAMPLE_ETH_PHY_GENERIC'"

.github/workflows/mdns__host-tests.yml

@@ -11,8 +11,11 @@ jobs:
   host_test_mdns:
     if: contains(github.event.pull_request.labels.*.name, 'mdns') || github.event_name == 'push'
     name: Host test build
+    strategy:
+      matrix:
+        idf_ver: ["latest", "release-v5.3", "release-v5.4", "release-v5.5"]
     runs-on: ubuntu-22.04
-    container: espressif/idf:release-v5.3
+    container: espressif/idf:${{ matrix.idf_ver }}
 
     steps:
       - name: Checkout esp-protocols

components/mdns/.cz.yaml

@@ -3,6 +3,6 @@ commitizen:
   bump_message: 'bump(mdns): $current_version -> $new_version'
   pre_bump_hooks: python ../../ci/changelog.py mdns
   tag_format: mdns-v$version
-  version: 1.11.3
+  version: 1.12.0
   version_files:
   - idf_component.yml

components/mdns/CHANGELOG.md

@@ -1,5 +1,32 @@
 # Changelog
 
+## [1.12.0](https://github.com/espressif/esp-protocols/commits/mdns-v1.12.0)
+
+### Features
+
+- add hostname change callback ([c2cd90b0](https://github.com/espressif/esp-protocols/commit/c2cd90b0))
+- Make mDNS browse feature optional ([2e0c2540](https://github.com/espressif/esp-protocols/commit/2e0c2540))
+
+### Bug Fixes
+
+- Update supported/tested IDF version matrix ([e8427217](https://github.com/espressif/esp-protocols/commit/e8427217))
+- Fix mdns browse SYNC vs. END race ([1b83e978](https://github.com/espressif/esp-protocols/commit/1b83e978))
+- Only depend on esp_wifi when Wi-Fi is enabled ([47724b7d](https://github.com/espressif/esp-protocols/commit/47724b7d), [#835](https://github.com/espressif/esp-protocols/issues/835))
+- Update host test to check goodbye and browse dupl ([ab03980d](https://github.com/espressif/esp-protocols/commit/ab03980d))
+- Fix mdns_browse_new() return dangling pointer on duplicated browsers ([e063449c](https://github.com/espressif/esp-protocols/commit/e063449c))
+- Create static task only when SPIRAM-task-alloc is ON ([6f31f27f](https://github.com/espressif/esp-protocols/commit/6f31f27f))
+- validate subtype label position ([a09b24fe](https://github.com/espressif/esp-protocols/commit/a09b24fe))
+- ignore subtype SRV and TXT questions ([7910501d](https://github.com/espressif/esp-protocols/commit/7910501d))
+- distinguish subtype queries from instances ([b9103169](https://github.com/espressif/esp-protocols/commit/b9103169))
+- Don't free a browse result already freed by an earlier sync batch ([9057f2d9](https://github.com/espressif/esp-protocols/commit/9057f2d9))
+
+### Updated
+
+- test(mdns): add hostname callback target test ([4c8b76dd](https://github.com/espressif/esp-protocols/commit/4c8b76dd))
+- test(mdns): add hostname callback host test ([1f35d5f3](https://github.com/espressif/esp-protocols/commit/1f35d5f3))
+- test(mdns): cover subtype query handling ([c14ed4fc](https://github.com/espressif/esp-protocols/commit/c14ed4fc))
+- refactor(mdns): share debug name formatting ([8c7bc865](https://github.com/espressif/esp-protocols/commit/8c7bc865))
+
 ## [1.11.3](https://github.com/espressif/esp-protocols/commits/mdns-v1.11.3)
 
 ### Features

components/mdns/idf_component.yml

@@ -1,4 +1,4 @@
-version: "1.11.3"
+version: "1.12.0"
 description: "Multicast UDP service used to provide local network service and host discovery."
 url: "https://github.com/espressif/esp-protocols/tree/master/components/mdns"
 issues: "https://github.com/espressif/esp-protocols/issues"

components/mdns/mdns_browser.c

@@ -56,6 +56,23 @@ static void browse_item_free(mdns_browse_t *browse)
     mdns_mem_free(browse);
 }
 
+/**
+ * @brief Check that a browse is still linked in @c s_browse
+ *
+ * Sync batches borrow @c browse_sync->browse. ACTION_BROWSE_END may detach and
+ * free that browse while a later ACTION_BROWSE_SYNC is still queued, so the
+ * sync handler must not touch the pointer without checking.
+ */
+static bool browse_is_in_list(const mdns_browse_t *browse)
+{
+    for (const mdns_browse_t *b = s_browse; b != NULL; b = b->next) {
+        if (b == browse) {
+            return true;
+        }
+    }
+    return false;
+}
+
 /**
  * @brief Check that a result node is still linked in the browse cache
  *
@@ -85,6 +102,10 @@ static bool result_is_cached(const mdns_browse_t *browse, const mdns_result_t *r
 static void browse_sync(mdns_browse_sync_t *browse_sync)
 {
     mdns_browse_t *browse = browse_sync->browse;
+    // END may have already detached+freed this browse, or delete marked it off
+    if (!browse_is_in_list(browse) || browse->state != BROWSE_RUNNING) {
+        return;
+    }
     mdns_browse_result_sync_t *sync_result = browse_sync->sync_result;
     while (sync_result) {
         mdns_result_t *result = sync_result->result;

components/mdns/tests/host_test/main/main.c

@@ -4,6 +4,8 @@
  * SPDX-License-Identifier: Unlicense OR CC0-1.0
  */
 #include <stdio.h>
+#include <stdlib.h>
+#include "esp_idf_version.h"
 #include "esp_log.h"
 #include "freertos/FreeRTOS.h"
 #include "freertos/task.h"
@@ -20,8 +22,15 @@ static EventGroupHandle_t s_exit_signal = NULL;
 
 static int exit_console(int argc, char **argv)
 {
+#ifdef CONFIG_IDF_TARGET_LINUX
+    /* Host tests do not need graceful REPL teardown; exit hard so leftover
+     * console_repl / linenoise threads cannot keep the process alive. */
+    ESP_LOGI(TAG, "Exit");
+    exit(0);
+#else
     xEventGroupSetBits(s_exit_signal, 1);
     return 0;
+#endif
 }
 
 #else
@@ -109,12 +118,16 @@ static void mdns_test_app(esp_netif_t *interface)
 #ifdef CONFIG_TEST_CONSOLE
     esp_console_repl_t *repl = NULL;
     esp_console_repl_config_t repl_config = ESP_CONSOLE_REPL_CONFIG_DEFAULT();
-    esp_console_dev_uart_config_t uart_config = ESP_CONSOLE_DEV_UART_CONFIG_DEFAULT();
     s_exit_signal = xEventGroupCreate();
 
     repl_config.prompt = "mdns>";
     // init console REPL environment
+#if CONFIG_IDF_TARGET_LINUX && ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(6, 1, 0)
+    ESP_ERROR_CHECK(esp_console_new_repl_stdio(&repl_config, &repl));
+#else
+    esp_console_dev_uart_config_t uart_config = ESP_CONSOLE_DEV_UART_CONFIG_DEFAULT();
     ESP_ERROR_CHECK(esp_console_new_repl_uart(&uart_config, &repl_config, &repl));
+#endif
 
     const esp_console_cmd_t cmd_exit = {
         .command = "exit",