From 7299060b016dbc423102fa8cdac2d9c71d2d4690 Mon Sep 17 00:00:00 2001 From: srgg Date: Wed, 28 Oct 2026 14:51:09 -0600 Subject: [PATCH 1/2] Change service visibility on the host task ble_gatts_svc_set_visibility() relinks a service's attributes between the host's attribute lists without ble_hs_lock(), while the host task walks the same list to answer ATT requests. removeService() called it from the application task. It now runs on the host task: a caller on another task posts it to the host's event queue and waits; the host task itself, and any caller before sync, call it directly. --- src/NimBLEDevice.cpp | 3 +++ src/NimBLEDevice.h | 1 + src/NimBLEServer.cpp | 46 +++++++++++++++++++++++++++++++++++++++++++- src/NimBLEServer.h | 1 + 4 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/NimBLEDevice.cpp b/src/NimBLEDevice.cpp index 4d1e40c3..92ea5782 100644 --- a/src/NimBLEDevice.cpp +++ b/src/NimBLEDevice.cpp @@ -99,6 +99,7 @@ std::array NimBLEDevice::m_pClie std::atomic NimBLEDevice::m_initialized{false}; uint32_t NimBLEDevice::m_passkey{123456}; std::atomic NimBLEDevice::m_synced{false}; +std::atomic NimBLEDevice::m_hostTask{nullptr}; ble_gap_event_listener NimBLEDevice::m_listener{}; std::vector NimBLEDevice::m_whiteList{}; uint8_t NimBLEDevice::m_ownAddrType{BLE_OWN_ADDR_PUBLIC}; @@ -885,7 +886,9 @@ void NimBLEDevice::onSync(void) { */ void NimBLEDevice::host_task(void* param) { NIMBLE_LOGI(LOG_TAG, "NimBLE Started!"); + m_hostTask = ble_npl_get_current_task_id(); nimble_port_run(); // This function will return only when nimble_port_stop() is executed + m_hostTask = nullptr; nimble_port_freertos_deinit(); } // host_task diff --git a/src/NimBLEDevice.h b/src/NimBLEDevice.h index b4fefe22..613b2346 100644 --- a/src/NimBLEDevice.h +++ b/src/NimBLEDevice.h @@ -219,6 +219,7 @@ class NimBLEDevice { private: static std::atomic m_synced; + static std::atomic m_hostTask; static std::atomic m_initialized; static uint32_t m_passkey; static ble_gap_event_listener m_listener; diff --git a/src/NimBLEServer.cpp b/src/NimBLEServer.cpp index 3aed0c6e..95058a6c 100644 --- a/src/NimBLEServer.cpp +++ b/src/NimBLEServer.cpp @@ -28,9 +28,11 @@ # ifdef USING_NIMBLE_ARDUINO_HEADERS # include "nimble/nimble/host/services/gap/include/services/gap/ble_svc_gap.h" # include "nimble/nimble/host/services/gatt/include/services/gatt/ble_svc_gatt.h" +# include "nimble/porting/nimble/include/nimble/nimble_port.h" # else # include "services/gap/ble_svc_gap.h" # include "services/gatt/ble_svc_gatt.h" +# include "nimble/nimble_port.h" # endif # define NIMBLE_SERVER_GET_PEER_NAME_ON_CONNECT_CB 0 @@ -837,7 +839,7 @@ void NimBLEServer::removeService(NimBLEService* service, bool deleteSvc) { return; } - int rc = ble_gatts_svc_set_visibility(service->getHandle(), 0); + int rc = setServiceVisibility(service->getHandle(), false); if (rc != 0) { return; } @@ -849,6 +851,48 @@ void NimBLEServer::removeService(NimBLEService* service, bool deleteSvc) { # endif } // removeService +/** + * @brief Show or hide a service on the host task. + * @details ble_gatts_svc_set_visibility() relinks the service's attributes without a lock while the + * host task walks the same list to answer ATT requests; a caller on another task waits for the host. + * @param [in] svcHandle The handle of the service. + * @param [in] visible True to show the service, false to hide it. + * @return 0 on success, BLE_HS_ENOENT if no service has that handle. + */ +int NimBLEServer::setServiceVisibility(uint16_t svcHandle, bool visible) { + // Before sync no peer can be connected, so nothing walks the list. + if (!NimBLEDevice::m_synced.load(std::memory_order_acquire) || + ble_npl_get_current_task_id() == NimBLEDevice::m_hostTask) { + return ble_gatts_svc_set_visibility(svcHandle, visible); + } + + struct VisibilityRequest { + uint16_t handle; + bool visible; + int rc; + ble_npl_sem done; + ble_npl_event ev; + } req{svcHandle, visible, 0, {}, {}}; + + if (ble_npl_sem_init(&req.done, 0) != BLE_NPL_OK) { + NIMBLE_LOGE(LOG_TAG, "Failed to initialize semaphore for service visibility"); + return BLE_HS_ENOMEM; + } + ble_npl_event_init( + &req.ev, + [](ble_npl_event* ev) { + auto* r = static_cast(ble_npl_event_get_arg(ev)); + r->rc = ble_gatts_svc_set_visibility(r->handle, r->visible); + ble_npl_sem_release(&r->done); + }, + &req); + ble_npl_eventq_put(nimble_port_get_dflt_eventq(), &req.ev); + ble_npl_sem_pend(&req.done, BLE_NPL_TIME_FOREVER); + ble_npl_event_deinit(&req.ev); + ble_npl_sem_deinit(&req.done); + return req.rc; +} // setServiceVisibility + /** * @brief Adds a service which was either already created but removed from availability,\n * or created and later added to services list. diff --git a/src/NimBLEServer.h b/src/NimBLEServer.h index 1260750c..b5add08b 100644 --- a/src/NimBLEServer.h +++ b/src/NimBLEServer.h @@ -127,6 +127,7 @@ class NimBLEServer { static void gattRegisterCallback(struct ble_gatt_register_ctxt* ctxt, void* arg); void setServiceChanged(); bool resetGATT(); + static int setServiceVisibility(uint16_t svcHandle, bool visible); bool m_gattsStarted : 1; bool m_svcChanged : 1; From 063c18cd495e5a18f054898216ffea1797a78d6f Mon Sep 17 00:00:00 2001 From: srgg Date: Wed, 28 Oct 2026 18:37:43 -0600 Subject: [PATCH 2/2] Refuse a queued visibility change once the host stops nimble_port_run() runs queued events only up to its stop event, and deinit() cleared m_synced only after nimble_port_stop() returned, so a removeService() racing deinit() could queue its event behind the stop and wait forever. deinit() now marks the host as stopping before the stop is queued; setServiceVisibility() checks the mark and queues under the same lock, so its event runs before the stop or is refused with BLE_HS_EOS. The event handler is now a named, documented function. --- src/NimBLEDevice.cpp | 13 +++++++++++ src/NimBLEDevice.h | 2 ++ src/NimBLEServer.cpp | 52 +++++++++++++++++++++++++++++--------------- src/NimBLEServer.h | 1 + 4 files changed, 51 insertions(+), 17 deletions(-) diff --git a/src/NimBLEDevice.cpp b/src/NimBLEDevice.cpp index 92ea5782..3abed522 100644 --- a/src/NimBLEDevice.cpp +++ b/src/NimBLEDevice.cpp @@ -100,6 +100,8 @@ std::atomic NimBLEDevice::m_initialized{false}; uint32_t NimBLEDevice::m_passkey{123456}; std::atomic NimBLEDevice::m_synced{false}; std::atomic NimBLEDevice::m_hostTask{nullptr}; +bool NimBLEDevice::m_hostStopping{false}; +std::atomic_flag NimBLEDevice::m_hostStopLock = ATOMIC_FLAG_INIT; ble_gap_event_listener NimBLEDevice::m_listener{}; std::vector NimBLEDevice::m_whiteList{}; uint8_t NimBLEDevice::m_ownAddrType{BLE_OWN_ADDR_PUBLIC}; @@ -886,6 +888,10 @@ void NimBLEDevice::onSync(void) { */ void NimBLEDevice::host_task(void* param) { NIMBLE_LOGI(LOG_TAG, "NimBLE Started!"); + { + NimBLEUtils::SpinGuard guard(m_hostStopLock); + m_hostStopping = false; + } m_hostTask = ble_npl_get_current_task_id(); nimble_port_run(); // This function will return only when nimble_port_stop() is executed m_hostTask = nullptr; @@ -1040,6 +1046,11 @@ bool NimBLEDevice::init(const std::string& deviceName) { bool NimBLEDevice::deinit(bool clearAll) { int rc = 0; if (isInitialized()) { + { + // Set before the stop event is queued: work queued after it never runs. + NimBLEUtils::SpinGuard guard(m_hostStopLock); + m_hostStopping = true; + } rc = nimble_port_stop(); if (rc == 0) { m_initialized.store(false); @@ -1062,6 +1073,8 @@ bool NimBLEDevice::deinit(bool clearAll) { # endif } else { NIMBLE_LOGE(LOG_TAG, "nimble_port_stop() failed with error: %d", rc); + NimBLEUtils::SpinGuard guard(m_hostStopLock); + m_hostStopping = false; return false; } } diff --git a/src/NimBLEDevice.h b/src/NimBLEDevice.h index 613b2346..0feffbc4 100644 --- a/src/NimBLEDevice.h +++ b/src/NimBLEDevice.h @@ -220,6 +220,8 @@ class NimBLEDevice { private: static std::atomic m_synced; static std::atomic m_hostTask; + static bool m_hostStopping; + static std::atomic_flag m_hostStopLock; static std::atomic m_initialized; static uint32_t m_passkey; static ble_gap_event_listener m_listener; diff --git a/src/NimBLEServer.cpp b/src/NimBLEServer.cpp index 95058a6c..d512c157 100644 --- a/src/NimBLEServer.cpp +++ b/src/NimBLEServer.cpp @@ -35,6 +35,17 @@ # include "nimble/nimble_port.h" # endif +namespace { +// A visibility change waiting for the host task; lives on the waiting caller's stack. +struct VisibilityRequest { + uint16_t handle; + bool visible; + int rc; + ble_npl_sem done; + ble_npl_event ev; +}; +} // namespace + # define NIMBLE_SERVER_GET_PEER_NAME_ON_CONNECT_CB 0 # define NIMBLE_SERVER_GET_PEER_NAME_ON_AUTH_CB 1 @@ -866,33 +877,40 @@ int NimBLEServer::setServiceVisibility(uint16_t svcHandle, bool visible) { return ble_gatts_svc_set_visibility(svcHandle, visible); } - struct VisibilityRequest { - uint16_t handle; - bool visible; - int rc; - ble_npl_sem done; - ble_npl_event ev; - } req{svcHandle, visible, 0, {}, {}}; - + VisibilityRequest req{svcHandle, visible, 0, {}, {}}; if (ble_npl_sem_init(&req.done, 0) != BLE_NPL_OK) { NIMBLE_LOGE(LOG_TAG, "Failed to initialize semaphore for service visibility"); return BLE_HS_ENOMEM; } - ble_npl_event_init( - &req.ev, - [](ble_npl_event* ev) { - auto* r = static_cast(ble_npl_event_get_arg(ev)); - r->rc = ble_gatts_svc_set_visibility(r->handle, r->visible); - ble_npl_sem_release(&r->done); - }, - &req); - ble_npl_eventq_put(nimble_port_get_dflt_eventq(), &req.ev); + ble_npl_event_init(&req.ev, setServiceVisibilityEvent, &req); + + { + // Queued ahead of the host's stop event, or not at all. + NimBLEUtils::SpinGuard guard(NimBLEDevice::m_hostStopLock); + if (NimBLEDevice::m_hostStopping) { + ble_npl_event_deinit(&req.ev); + ble_npl_sem_deinit(&req.done); + return BLE_HS_EOS; + } + ble_npl_eventq_put(nimble_port_get_dflt_eventq(), &req.ev); + } + ble_npl_sem_pend(&req.done, BLE_NPL_TIME_FOREVER); ble_npl_event_deinit(&req.ev); ble_npl_sem_deinit(&req.done); return req.rc; } // setServiceVisibility +/** + * @brief Host task handler for a visibility change queued by setServiceVisibility(). + * @param [in] ev The event; its argument is the waiting caller's VisibilityRequest. + */ +void NimBLEServer::setServiceVisibilityEvent(ble_npl_event* ev) { + auto* req = static_cast(ble_npl_event_get_arg(ev)); + req->rc = ble_gatts_svc_set_visibility(req->handle, req->visible); + ble_npl_sem_release(&req->done); +} // setServiceVisibilityEvent + /** * @brief Adds a service which was either already created but removed from availability,\n * or created and later added to services list. diff --git a/src/NimBLEServer.h b/src/NimBLEServer.h index b5add08b..02dcef40 100644 --- a/src/NimBLEServer.h +++ b/src/NimBLEServer.h @@ -128,6 +128,7 @@ class NimBLEServer { void setServiceChanged(); bool resetGATT(); static int setServiceVisibility(uint16_t svcHandle, bool visible); + static void setServiceVisibilityEvent(struct ble_npl_event* ev); bool m_gattsStarted : 1; bool m_svcChanged : 1;