Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/NimBLEClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,12 @@ bool NimBLEClient::connect(const NimBLEAddress& address, bool deleteAttributes,
goto error;
}

if (!asyncConnect && NimBLEUtils::inHostTask()) {
NIMBLE_LOGE(LOG_TAG, "Cannot connect synchronously from host task");
rc = BLE_HS_EREJECT;
goto error;
}

m_connStatus = CONNECTING;
m_peerAddress = address;
m_config.asyncConnect = asyncConnect;
Expand Down Expand Up @@ -334,6 +340,11 @@ bool NimBLEClient::connect(const NimBLEAddress& address, bool deleteAttributes,
*/
bool NimBLEClient::secureConnection(bool async) const {
NIMBLE_LOGD(LOG_TAG, ">> secureConnection()");
if (!async && NimBLEUtils::inHostTask()) {
NIMBLE_LOGE(LOG_TAG, "Cannot secure connection synchronously from host task");
m_lastErr = BLE_HS_EREJECT;
return false;
}

int rc = 0;
if (async && !NimBLEDevice::startSecurity(m_connHandle, &rc)) {
Expand Down Expand Up @@ -669,6 +680,10 @@ NimBLERemoteService* NimBLEClient::getService(const char* uuid) {
*/
NimBLERemoteService* NimBLEClient::getService(const NimBLEUUID& uuid) {
NIMBLE_LOGD(LOG_TAG, ">> getService: uuid: %s", uuid.toString().c_str());
if (NimBLEUtils::inHostTask()) {
NIMBLE_LOGE(LOG_TAG, "getService cannot be called from host task");
return nullptr;
Comment on lines +683 to +685
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Arrr, cached services should still be reachable.

This now rejects getService() before the cache scan on Lines 688-693. That breaks host-task callbacks that only need a previously discovered service even though no blocking call would occur. Move the guard to just before the first retrieveServices() call so only cache misses are rejected.

⚓ Proposed fix
 NimBLERemoteService* NimBLEClient::getService(const NimBLEUUID& uuid) {
     NIMBLE_LOGD(LOG_TAG, ">> getService: uuid: %s", uuid.toString().c_str());
-    if (NimBLEUtils::inHostTask()) {
-        NIMBLE_LOGE(LOG_TAG, "getService cannot be called from host task");
-        return nullptr;
-    }
 
     for (auto& it : m_svcVec) {
         if (it->getUUID() == uuid) {
             NIMBLE_LOGD(LOG_TAG, "<< getService: found the service with uuid: %s", uuid.toString().c_str());
             return it;
         }
     }
+
+    if (NimBLEUtils::inHostTask()) {
+        NIMBLE_LOGE(LOG_TAG, "getService cannot discover from host task");
+        return nullptr;
+    }
 
     size_t prevSize = m_svcVec.size();
     if (retrieveServices(&uuid)) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/NimBLEClient.cpp` around lines 683 - 685, The guard that rejects calls
from the host task (NimBLEUtils::inHostTask()) is currently placed at the start
of getService(), preventing access to cached services; move that inHostTask()
check so it runs immediately before the first call to retrieveServices() (i.e.,
only when a cache miss would trigger a blocking retrieval), and leave the
early-path that returns a cached service intact so host-task callbacks can still
return previously discovered services from the cache via getService().

}

for (auto& it : m_svcVec) {
if (it->getUUID() == uuid) {
Expand Down
1 change: 1 addition & 0 deletions src/NimBLEDevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,7 @@ void NimBLEDevice::onSync(void) {
*/
void NimBLEDevice::host_task(void* param) {
NIMBLE_LOGI(LOG_TAG, "NimBLE Started!");
NimBLEUtils::m_hostTaskHandle = ble_npl_get_current_task_id();
nimble_port_run(); // This function will return only when nimble_port_stop() is executed
nimble_port_freertos_deinit();
Comment on lines +879 to 881
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C2 'm_hostTaskHandle|inHostTask\s*\(' src

Repository: h2zero/esp-nimble-cpp

Length of output: 5298


Arr, swab the deck—reset m_hostTaskHandle when the host task be walkin' the plank!

This bit o' code be stashin' the host-task ID at startup but never clairin' it when nimble_port_run() returns, ye scallywag! If th' NPL/RTOS be reusinn' that task handle later, unrelated callers might start trippin' inHostTask() checks and gettin' spurious early rejects after deinit.

⚓ Proposed fix
 void NimBLEDevice::host_task(void* param) {
     NIMBLE_LOGI(LOG_TAG, "NimBLE Started!");
     NimBLEUtils::m_hostTaskHandle = ble_npl_get_current_task_id();
     nimble_port_run(); // This function will return only when nimble_port_stop() is executed
+    NimBLEUtils::m_hostTaskHandle = nullptr;
     nimble_port_freertos_deinit();
 } // host_task
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
NimBLEUtils::m_hostTaskHandle = ble_npl_get_current_task_id();
nimble_port_run(); // This function will return only when nimble_port_stop() is executed
nimble_port_freertos_deinit();
NimBLEUtils::m_hostTaskHandle = ble_npl_get_current_task_id();
nimble_port_run(); // This function will return only when nimble_port_stop() is executed
NimBLEUtils::m_hostTaskHandle = nullptr;
nimble_port_freertos_deinit();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/NimBLEDevice.cpp` around lines 879 - 881, The host task ID stored in
NimBLEUtils::m_hostTaskHandle is set before calling nimble_port_run() but never
cleared when nimble_port_run() returns, causing inHostTask() to give false
positives after deinit; fix by resetting NimBLEUtils::m_hostTaskHandle to an
invalid value (e.g., 0/null) once nimble_port_run() returns and before/after
calling nimble_port_freertos_deinit() so the stored task handle is not reused by
unrelated code.

} // host_task
Expand Down
5 changes: 5 additions & 0 deletions src/NimBLERemoteCharacteristic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,11 @@ bool NimBLERemoteCharacteristic::retrieveDescriptors(NimBLEDescriptorFilter* pFi
*/
NimBLERemoteDescriptor* NimBLERemoteCharacteristic::getDescriptor(const NimBLEUUID& uuid) const {
NIMBLE_LOGD(LOG_TAG, ">> getDescriptor: uuid: %s", uuid.toString().c_str());
if (NimBLEUtils::inHostTask()) {
NIMBLE_LOGE(LOG_TAG, "getDescriptor cannot be called from host task");
return nullptr;
Comment on lines +158 to +160
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Arrr, let cached descriptors pass; only block discovery.

This return fires before the cache scan on Lines 167-172, so even an already discovered descriptor now looks missing from host-task callbacks. Downstream, setNotify() on Lines 249-257 treats that nullptr as “CCCD not found” and returns true, so a cached CCCD becomes a silent no-op. Move the guard to just before retrieveDescriptors().

⚓ Proposed fix
 NimBLERemoteDescriptor* NimBLERemoteCharacteristic::getDescriptor(const NimBLEUUID& uuid) const {
     NIMBLE_LOGD(LOG_TAG, ">> getDescriptor: uuid: %s", uuid.toString().c_str());
-    if (NimBLEUtils::inHostTask()) {
-        NIMBLE_LOGE(LOG_TAG, "getDescriptor cannot be called from host task");
-        return nullptr;
-    }
 
     NimBLEUUID             uuidTmp{uuid};
     NimBLETaskData         taskData(const_cast<NimBLERemoteCharacteristic*>(this));
     NimBLEDescriptorFilter filter{nullptr, &uuidTmp, &taskData};
 
     for (const auto& dsc : m_vDescriptors) {
         if (dsc->getUUID() == uuid) {
             filter.dsc = dsc;
             goto Done;
         }
     }
+
+    if (NimBLEUtils::inHostTask()) {
+        NIMBLE_LOGE(LOG_TAG, "getDescriptor cannot discover from host task");
+        goto Done;
+    }
 
     if (!retrieveDescriptors(&filter) || filter.dsc) {
         goto Done;
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/NimBLERemoteCharacteristic.cpp` around lines 158 - 160, The guard that
blocks host-task context is placed too early in getDescriptor(), preventing
cached descriptors from being returned; move the NimBLEUtils::inHostTask() check
so it happens immediately before calling retrieveDescriptors() (i.e., keep early
cache-scan logic in getDescriptor() intact, then if a discovery is required call
the inHostTask() guard before invoking retrieveDescriptors()); update references
in getDescriptor() and ensure setNotify() still treats a found cached CCCD
correctly (no change to setNotify() logic required).

}

NimBLEUUID uuidTmp{uuid};
NimBLETaskData taskData(const_cast<NimBLERemoteCharacteristic*>(this));
NimBLEDescriptorFilter filter{nullptr, &uuidTmp, &taskData};
Expand Down
10 changes: 10 additions & 0 deletions src/NimBLERemoteService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ NimBLERemoteCharacteristic* NimBLERemoteService::getCharacteristic(const char* u
*/
NimBLERemoteCharacteristic* NimBLERemoteService::getCharacteristic(const NimBLEUUID& uuid) const {
NIMBLE_LOGD(LOG_TAG, ">> getCharacteristic: uuid: %s", uuid.toString().c_str());
if (NimBLEUtils::inHostTask()) {
NIMBLE_LOGE(LOG_TAG, "getCharacteristic cannot be called from host task");
return nullptr;
Comment on lines +78 to +80
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Arrr, don’t sink cached characteristic lookups.

This guard runs before the cache scan on Lines 85-91, so host-task callbacks can no longer fetch an already discovered characteristic even though no blocking call would occur. Gate only retrieveCharacteristics() here.

⚓ Proposed fix
 NimBLERemoteCharacteristic* NimBLERemoteService::getCharacteristic(const NimBLEUUID& uuid) const {
     NIMBLE_LOGD(LOG_TAG, ">> getCharacteristic: uuid: %s", uuid.toString().c_str());
-    if (NimBLEUtils::inHostTask()) {
-        NIMBLE_LOGE(LOG_TAG, "getCharacteristic cannot be called from host task");
-        return nullptr;
-    }
 
     NimBLERemoteCharacteristic* pChar     = nullptr;
 
     for (const auto& it : m_vChars) {
         if (it->getUUID() == uuid) {
             pChar = it;
             NIMBLE_LOGD(LOG_TAG, "<< getCharacteristic: found in cache");
             return pChar;
         }
     }
+
+    if (NimBLEUtils::inHostTask()) {
+        NIMBLE_LOGE(LOG_TAG, "getCharacteristic cannot discover from host task");
+        return nullptr;
+    }
 
     if (retrieveCharacteristics(&uuid, &pChar) && pChar == nullptr) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/NimBLERemoteService.cpp` around lines 78 - 80, The current early-return
in getCharacteristic when NimBLEUtils::inHostTask() is true prevents returning
cached characteristics; instead, only prevent calling the blocking
retrieveCharacteristics(). Modify getCharacteristic so it still scans the
existing cache (the loop that checks cached characteristics) even when
inHostTask() is true, but conditionally skip or gate the call to
retrieveCharacteristics() (and log if desired) when inHostTask() to avoid
blocking the host task; reference functions/methods: getCharacteristic(),
retrieveCharacteristics(), and NimBLEUtils::inHostTask() to locate and implement
this change.

}

NimBLERemoteCharacteristic* pChar = nullptr;

for (const auto& it : m_vChars) {
Expand Down Expand Up @@ -116,6 +121,11 @@ NimBLERemoteCharacteristic* NimBLERemoteService::getCharacteristic(const NimBLEU
* @return A read-only reference to the vector of characteristics retrieved for this service.
*/
const std::vector<NimBLERemoteCharacteristic*>& NimBLERemoteService::getCharacteristics(bool refresh) const {
if (refresh && NimBLEUtils::inHostTask()) {
NIMBLE_LOGE(LOG_TAG, "cannot refresh characteristics from host task");
return m_vChars;
}

if (refresh) {
deleteCharacteristics();
retrieveCharacteristics();
Expand Down
9 changes: 9 additions & 0 deletions src/NimBLERemoteValueAttribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ bool NimBLERemoteValueAttribute::writeValue(const uint8_t* data, size_t length,
goto Done;
}

if (NimBLEUtils::inHostTask()) {
NIMBLE_LOGE(LOG_TAG, "writeValue cannot be called from the host task");
return false;
}

do {
if (length > mtu) {
NIMBLE_LOGI(LOG_TAG, "writeValue: long write");
Expand Down Expand Up @@ -123,6 +128,10 @@ int NimBLERemoteValueAttribute::onWriteCB(uint16_t conn_handle, const ble_gatt_e
*/
NimBLEAttValue NimBLERemoteValueAttribute::readValue(time_t* timestamp) {
NIMBLE_LOGD(LOG_TAG, ">> readValue()");
if (NimBLEUtils::inHostTask()) {
NIMBLE_LOGE(LOG_TAG, "readValue cannot be called from the host task");
return NimBLEAttValue();
}

NimBLEAttValue value{};
const NimBLEClient* pClient = getClient();
Expand Down
5 changes: 5 additions & 0 deletions src/NimBLEScan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,11 @@ NimBLEScanResults NimBLEScan::getResults(uint32_t duration, bool is_continue) {
return m_scanResults;
}

if (NimBLEUtils::inHostTask()) {
NIMBLE_LOGE(LOG_TAG, "Cannot call blocking getResults from NimBLE host task");
return m_scanResults;
}

NimBLETaskData taskData;
m_pTaskData = &taskData;

Expand Down
19 changes: 19 additions & 0 deletions src/NimBLEUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@
constexpr uint32_t TASK_BLOCK_BIT = (1 << MYNEWT_VAL(NIMBLE_CPP_FREERTOS_TASK_BLOCK_BIT));
static const char* LOG_TAG = "NimBLEUtils";

void* NimBLEUtils::m_hostTaskHandle = nullptr;

/**
* @brief Construct a NimBLETaskData instance.
* @param [in] pInstance An instance of the class that will be waiting.
Expand Down Expand Up @@ -604,4 +606,21 @@ NimBLEAddress NimBLEUtils::generateAddr(bool nrpa) {
return NimBLEAddress{addr};
} // generateAddr


/**
* @brief Get the handle of the task that is running the NimBLE host.
* @return The task handle or nullptr if there was an error.
*/
void* NimBLEUtils::getHostTaskHandle() {
return m_hostTaskHandle;
} // getHostTaskHandle

/**
* @brief Check if the current task is the NimBLE host task.
* @return True if the current task is the host task, false otherwise.
*/
bool NimBLEUtils::inHostTask() {
return ble_npl_get_current_task_id() == NimBLEUtils::getHostTaskHandle();
}

#endif // CONFIG_BT_NIMBLE_ENABLED
29 changes: 18 additions & 11 deletions src/NimBLEUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,18 @@
# endif
# endif

#if MYNEWT_VAL(NIMBLE_CPP_DEBUG_ASSERT_ENABLED) && !defined NDEBUG
void nimble_cpp_assert(const char *file, unsigned line) __attribute((weak, noreturn));
# define NIMBLE_ATT_VAL_FILE (__builtin_strrchr(__FILE__, '/') ? \
__builtin_strrchr (__FILE__, '/') + 1 : __FILE__)
# define NIMBLE_CPP_DEBUG_ASSERT(cond) \
if (!(cond)) { \
nimble_cpp_assert(NIMBLE_ATT_VAL_FILE, __LINE__); \
}
#else
# define NIMBLE_CPP_DEBUG_ASSERT(cond) (void(0))
#endif
# if MYNEWT_VAL(NIMBLE_CPP_DEBUG_ASSERT_ENABLED) && !defined NDEBUG
void nimble_cpp_assert(const char* file, unsigned line) __attribute__((weak, noreturn));
# define NIMBLE_ATT_VAL_FILE (__builtin_strrchr(__FILE__, '/') ? __builtin_strrchr(__FILE__, '/') + 1 : __FILE__)
# define NIMBLE_CPP_DEBUG_ASSERT(cond) \
do { \
if (!(cond)) { \
nimble_cpp_assert(NIMBLE_ATT_VAL_FILE, __LINE__); \
} \
} while (0)
# else
# define NIMBLE_CPP_DEBUG_ASSERT(cond) (void(0))
# endif

# include <string>

Expand Down Expand Up @@ -74,6 +75,12 @@ class NimBLEUtils {
static NimBLEAddress generateAddr(bool nrpa);
static bool taskWait(const NimBLETaskData& taskData, uint32_t timeout);
static void taskRelease(const NimBLETaskData& taskData, int rc = 0);
static void* getHostTaskHandle();
static bool inHostTask();

private:
friend class NimBLEDevice;
static void* m_hostTaskHandle;
};

#endif // CONFIG_BT_NIMBLE_ENABLED
Expand Down