Skip to content

Commit 3e56e68

Browse files
committed
[Bugfix] NimBLEDevice::createServer can crash if stack not initialized.
* [Bugfix] NimBLEDevice::createServer can crash if stack not initialized. This removes the gatts/gap reset calls from NimBLEDevice::createServer so that it can be safely used before initializing the stack. This also deprecates NimBLEService::start the the services will now be added/created only when the server is started.
1 parent 4575ccb commit 3e56e68

12 files changed

Lines changed: 46 additions & 51 deletions

File tree

examples/Bluetooth_5/NimBLE_extended_server/NimBLE_extended_server.ino

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,6 @@ void setup() {
9898

9999
pCharacteristic->setValue("Hello World");
100100

101-
/** Start the service */
102-
pService->start();
103-
104101
/**
105102
* Create an extended advertisement with the instance ID 0 and set the PHY's.
106103
* Multiple instances can be added as long as the instance ID is incremented.

examples/Bluetooth_5/NimBLE_multi_advertiser/NimBLE_multi_advertiser.ino

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,6 @@ void setup() {
116116

117117
pCharacteristic->setValue("Hello World");
118118

119-
/** Start the service */
120-
pService->start();
121-
122119
/** Create our multi advertising instances */
123120

124121
/** extended scannable instance advertising on coded and 1m PHY's. */

examples/L2CAP/L2CAP_Server/L2CAP_Server.ino

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ void setup() {
7070
auto service = server->createService(SERVICE_UUID);
7171
auto characteristic = service->createCharacteristic(CHARACTERISTIC_UUID, NIMBLE_PROPERTY::READ);
7272
characteristic->setValue(L2CAP_CHANNEL);
73-
service->start();
7473

7574
auto advertising = BLEDevice::getAdvertising();
7675
advertising->addServiceUUID(SERVICE_UUID);

examples/NimBLE_Secure_Server/NimBLE_Secure_Server.ino

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ void setup() {
2929
pService->createCharacteristic("1235",
3030
NIMBLE_PROPERTY::READ | NIMBLE_PROPERTY::READ_ENC | NIMBLE_PROPERTY::READ_AUTHEN);
3131

32-
pService->start();
3332
pNonSecureCharacteristic->setValue("Hello Non Secure BLE");
3433
pSecureCharacteristic->setValue("Hello Secure BLE");
3534

examples/NimBLE_Server/NimBLE_Server.ino

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,6 @@ void setup(void) {
186186
pC01Ddsc->setValue("Send it back!");
187187
pC01Ddsc->setCallbacks(&dscCallbacks);
188188

189-
/** Start the services when finished creating all Characteristics and Descriptors */
190-
pDeadService->start();
191-
pBaadService->start();
192-
193189
/** Create an advertising instance and add the services to the advertised data */
194190
NimBLEAdvertising* pAdvertising = NimBLEDevice::getAdvertising();
195191
pAdvertising->setName("NimBLE-Server");

examples/NimBLE_Server_Whitelist/NimBLE_Server_Whitelist.ino

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ void setup() {
5555
pCharacteristic =
5656
pService->createCharacteristic(CHARACTERISTIC_UUID,
5757
NIMBLE_PROPERTY::READ | NIMBLE_PROPERTY::WRITE | NIMBLE_PROPERTY::NOTIFY);
58-
pService->start();
5958

6059
NimBLEAdvertising* pAdvertising = NimBLEDevice::getAdvertising();
6160
pAdvertising->addServiceUUID(SERVICE_UUID);

src/NimBLEAdvertising.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,9 @@ bool NimBLEAdvertising::start(uint32_t duration, const NimBLEAddress* dirAddr) {
197197

198198
# if CONFIG_BT_NIMBLE_ROLE_PERIPHERAL
199199
NimBLEServer* pServer = NimBLEDevice::getServer();
200-
if (pServer != nullptr) {
201-
pServer->start(); // make sure the GATT server is ready before advertising
200+
if (pServer != nullptr && !pServer->start()) { // make sure the GATT server is ready before advertising
201+
NIMBLE_LOGE(LOG_TAG, "Failed to start GATT server");
202+
return false;
202203
}
203204
# endif
204205

src/NimBLEDevice.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,6 @@ extern "C" int ble_vhci_disc_duplicate_mode_enable(int mode);
125125
NimBLEServer* NimBLEDevice::createServer() {
126126
if (NimBLEDevice::m_pServer == nullptr) {
127127
NimBLEDevice::m_pServer = new NimBLEServer();
128-
ble_gatts_reset();
129-
ble_svc_gap_init();
130-
ble_svc_gatt_init();
131128
}
132129

133130
return m_pServer;

src/NimBLEServer.cpp

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -254,14 +254,18 @@ void NimBLEServer::gattRegisterCallback(ble_gatt_register_ctxt* ctxt, void* arg)
254254
* @details Required to be called after setup of all services and characteristics / descriptors
255255
* for the NimBLE host to register them.
256256
*/
257-
void NimBLEServer::start() {
257+
bool NimBLEServer::start() {
258258
if (m_svcChanged && !getConnectedCount()) {
259259
NIMBLE_LOGD(LOG_TAG, "Services have changed since last start, resetting GATT server");
260-
resetGATT();
260+
m_gattsStarted = false;
261261
}
262262

263263
if (m_gattsStarted) {
264-
return; // already started
264+
return true; // already started
265+
}
266+
267+
if (!resetGATT()) {
268+
return false;
265269
}
266270

267271
ble_hs_cfg.gatts_register_cb = NimBLEServer::gattRegisterCallback;
@@ -271,7 +275,7 @@ void NimBLEServer::start() {
271275
int rc = ble_gatts_start();
272276
if (rc != 0) {
273277
NIMBLE_LOGE(LOG_TAG, "ble_gatts_start; rc=%d, %s", rc, NimBLEUtils::returnCodeToString(rc));
274-
return;
278+
return false;
275279
}
276280

277281
# if CONFIG_NIMBLE_CPP_LOG_LEVEL >= 4
@@ -298,6 +302,7 @@ void NimBLEServer::start() {
298302
}
299303

300304
m_gattsStarted = true;
305+
return true;
301306
} // start
302307

303308
/**
@@ -320,7 +325,7 @@ bool NimBLEServer::disconnect(uint16_t connHandle, uint8_t reason) const {
320325
* @brief Disconnect the specified client with optional reason.
321326
* @param [in] connInfo Connection of the client to disconnect.
322327
* @param [in] reason code for disconnecting.
323-
* @return NimBLE host return code.
328+
* @return True if successful.
324329
*/
325330
bool NimBLEServer::disconnect(const NimBLEConnInfo& connInfo, uint8_t reason) const {
326331
return disconnect(connInfo.getConnHandle(), reason);
@@ -490,7 +495,7 @@ int NimBLEServer::handleGapEvent(ble_gap_event* event, void* arg) {
490495

491496
peerInfo.m_desc = event->disconnect.conn;
492497
pServer->m_pServerCallbacks->onDisconnect(pServer, peerInfo, event->disconnect.reason);
493-
# if !CONFIG_BT_NIMBLE_EXT_ADV
498+
# if !CONFIG_BT_NIMBLE_EXT_ADV && CONFIG_BT_NIMBLE_ROLE_BROADCASTER
494499
if (pServer->m_advertiseOnDisconnect) {
495500
pServer->startAdvertising();
496501
}
@@ -857,8 +862,13 @@ void NimBLEServer::addService(NimBLEService* service) {
857862

858863
/**
859864
* @brief Resets the GATT server, used when services are added/removed after initialization.
865+
* @return True if successful.
866+
* @details This will reset the GATT server and re-register all services, characteristics, and
867+
* descriptors that have not been removed. Services, characteristics, and descriptors that have been
868+
* removed but not deleted will be skipped and have their handles cleared, and those that have been
869+
* deleted will be removed from the server's service vector.
860870
*/
861-
void NimBLEServer::resetGATT() {
871+
bool NimBLEServer::resetGATT() {
862872
# if CONFIG_BT_NIMBLE_ROLE_BROADCASTER
863873
NimBLEDevice::stopAdvertising();
864874
# endif
@@ -867,8 +877,6 @@ void NimBLEServer::resetGATT() {
867877
ble_svc_gap_init();
868878
ble_svc_gatt_init();
869879

870-
m_gattsStarted = false;
871-
872880
for (auto svcIt = m_svcVec.begin(); svcIt != m_svcVec.end();) {
873881
auto* pSvc = *svcIt;
874882
if (pSvc->getRemoved() == NIMBLE_ATT_REMOVE_DELETE) {
@@ -902,12 +910,17 @@ void NimBLEServer::resetGATT() {
902910
}
903911

904912
if (pSvc->getRemoved() == 0) {
905-
pSvc->start();
913+
if (!pSvc->start_internal()) {
914+
NIMBLE_LOGE(LOG_TAG, "Failed to start service: %s", pSvc->getUUID().toString().c_str());
915+
return false;
916+
}
906917
}
907918

908919
pSvc->m_handle = 0;
909920
++svcIt;
910921
}
922+
923+
return true;
911924
} // resetGATT
912925

913926
/**

src/NimBLEServer.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ class NimBLEClient;
6161
*/
6262
class NimBLEServer {
6363
public:
64-
void start();
64+
bool start();
6565
uint8_t getConnectedCount() const;
6666
bool disconnect(uint16_t connHandle, uint8_t reason = BLE_ERR_REM_USER_CONN_TERM) const;
6767
bool disconnect(const NimBLEConnInfo& connInfo, uint8_t reason = BLE_ERR_REM_USER_CONN_TERM) const;
@@ -122,7 +122,7 @@ class NimBLEServer {
122122
bool m_gattsStarted : 1;
123123
bool m_svcChanged : 1;
124124
bool m_deleteCallbacks : 1;
125-
# if !CONFIG_BT_NIMBLE_EXT_ADV
125+
# if !CONFIG_BT_NIMBLE_EXT_ADV && CONFIG_BT_NIMBLE_ROLE_BROADCASTER
126126
bool m_advertiseOnDisconnect : 1;
127127
# endif
128128
NimBLEServerCallbacks* m_pServerCallbacks;
@@ -137,7 +137,7 @@ class NimBLEServer {
137137
static int handleGattEvent(uint16_t connHandle, uint16_t attrHandle, ble_gatt_access_ctxt* ctxt, void* arg);
138138
static void gattRegisterCallback(struct ble_gatt_register_ctxt* ctxt, void* arg);
139139
void serviceChanged();
140-
void resetGATT();
140+
bool resetGATT();
141141

142142
}; // NimBLEServer
143143

0 commit comments

Comments
 (0)