summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSven Gothel <[email protected]>2020-09-29 18:08:19 +0200
committerSven Gothel <[email protected]>2020-09-29 18:08:19 +0200
commit9a0964148e93e14005c8f5425af794b736cdd10d (patch)
tree4e1ccf41000642f57463fbb98fcd26ebda7153ad
parent473aaae3080d859c22c640bbb903249f4fe007b7 (diff)
[L2CAP|HCI]Comm + [GATT|HCI]Handler/DBTManager: Properly synchronize reader thread lifecycle, disallow runaway reader threads
Reader threads of [GATT|HCI]Handler/DBTManager are detached to avoid program termination if these threads reach premature EOL, i.e. 'terminate called without an active exception'. However, no thread shall survive their owner lifecycle, i.e. [GATT|HCI]Handler/DBTManager, as they use their resources. Hence added synchronization on closing/disconnecting [GATT|HCI]Handler/DBTManager, waiting for the threads to end. This ensures that the resources used by the threads are available at all times during their life. +++ Enhanced HCIComm::close() and read() to interrupt operations and shorten closing time costs, similar to L2CAPComm. L2CAPComm added interruption on read operation as well.
-rw-r--r--api/direct_bt/DBTManager.hpp6
-rw-r--r--api/direct_bt/GATTHandler.hpp2
-rw-r--r--api/direct_bt/HCIComm.hpp8
-rw-r--r--api/direct_bt/HCIHandler.hpp4
-rw-r--r--api/direct_bt/L2CAPComm.hpp1
-rw-r--r--api/direct_bt/dbt_debug.hpp24
-rw-r--r--src/direct_bt/DBTManager.cpp83
-rw-r--r--src/direct_bt/GATTHandler.cpp62
-rw-r--r--src/direct_bt/HCIComm.cpp47
-rw-r--r--src/direct_bt/HCIHandler.cpp75
-rw-r--r--src/direct_bt/L2CAPComm.cpp38
11 files changed, 241 insertions, 109 deletions
diff --git a/api/direct_bt/DBTManager.hpp b/api/direct_bt/DBTManager.hpp
index 05f93aae..754a3a6b 100644
--- a/api/direct_bt/DBTManager.hpp
+++ b/api/direct_bt/DBTManager.hpp
@@ -168,13 +168,15 @@ namespace direct_bt {
HCIComm comm;
LFRingbuffer<std::shared_ptr<MgmtEvent>, nullptr> mgmtEventRing;
- std::thread mgmtReaderThread;
+ std::atomic<pthread_t> mgmtReaderThreadId;
std::atomic<bool> mgmtReaderRunning;
std::atomic<bool> mgmtReaderShallStop;
- std::mutex mtx_mgmtReaderInit;
+ std::mutex mtx_mgmtReaderLifecycle;
std::condition_variable cv_mgmtReaderInit;
std::recursive_mutex mtx_sendReply; // for sendWithReply
+ std::atomic<bool> allowClose;
+
/** One MgmtAdapterEventCallbackList per event type, allowing multiple callbacks to be invoked for each event */
std::array<MgmtAdapterEventCallbackList, static_cast<uint16_t>(MgmtEvent::Opcode::MGMT_EVENT_TYPE_COUNT)> mgmtAdapterEventCallbackLists;
std::recursive_mutex mtx_callbackLists;
diff --git a/api/direct_bt/GATTHandler.hpp b/api/direct_bt/GATTHandler.hpp
index 913355c0..2befd51e 100644
--- a/api/direct_bt/GATTHandler.hpp
+++ b/api/direct_bt/GATTHandler.hpp
@@ -165,7 +165,7 @@ namespace direct_bt {
std::atomic<pthread_t> l2capReaderThreadId;
std::atomic<bool> l2capReaderRunning;
std::atomic<bool> l2capReaderShallStop;
- std::mutex mtx_l2capReaderInit;
+ std::mutex mtx_l2capReaderLifecycle;
std::condition_variable cv_l2capReaderInit;
/** send immediate confirmation of indication events from device, defaults to true. */
diff --git a/api/direct_bt/HCIComm.hpp b/api/direct_bt/HCIComm.hpp
index 4d44dfb3..15af8881 100644
--- a/api/direct_bt/HCIComm.hpp
+++ b/api/direct_bt/HCIComm.hpp
@@ -59,13 +59,13 @@ namespace direct_bt {
static int hci_close_dev(int dd) noexcept;
std::recursive_mutex mtx_write;
- int socket_descriptor; // the hci socket
+ std::atomic<int> socket_descriptor; // the hci socket
+ std::atomic<bool> interrupt_flag; // for forced disconnect
+ std::atomic<pthread_t> tid_read;
public:
/** Constructing a newly opened HCI communication channel instance */
- HCIComm(const uint16_t dev_id, const uint16_t channel) noexcept
- : dev_id( dev_id ), channel( channel ),
- socket_descriptor( hci_open_dev(dev_id, channel) ) { }
+ HCIComm(const uint16_t dev_id, const uint16_t channel) noexcept;
/**
* Releases this instance after issuing {@link #close()}.
diff --git a/api/direct_bt/HCIHandler.hpp b/api/direct_bt/HCIHandler.hpp
index 9cb89add..ae4203bd 100644
--- a/api/direct_bt/HCIHandler.hpp
+++ b/api/direct_bt/HCIHandler.hpp
@@ -218,10 +218,12 @@ namespace direct_bt {
std::atomic<pthread_t> hciReaderThreadId;
std::atomic<bool> hciReaderRunning;
std::atomic<bool> hciReaderShallStop;
- std::mutex mtx_hciReaderInit;
+ std::mutex mtx_hciReaderLifecycle;
std::condition_variable cv_hciReaderInit;
std::recursive_mutex mtx_sendReply; // for sendWith*Reply, process*Command, ..
+ std::atomic<bool> allowClose;
+
std::vector<HCIConnectionRef> connectionList;
std::recursive_mutex mtx_connectionList;
/**
diff --git a/api/direct_bt/L2CAPComm.hpp b/api/direct_bt/L2CAPComm.hpp
index a0418cb7..572e6375 100644
--- a/api/direct_bt/L2CAPComm.hpp
+++ b/api/direct_bt/L2CAPComm.hpp
@@ -137,6 +137,7 @@ namespace direct_bt {
std::atomic<bool> has_ioerror; // reflects state
std::atomic<bool> interrupt_flag; // for forced disconnect
std::atomic<pthread_t> tid_connect;
+ std::atomic<pthread_t> tid_read;
public:
/**
diff --git a/api/direct_bt/dbt_debug.hpp b/api/direct_bt/dbt_debug.hpp
index 107a28e6..889a90f4 100644
--- a/api/direct_bt/dbt_debug.hpp
+++ b/api/direct_bt/dbt_debug.hpp
@@ -54,15 +54,31 @@ namespace direct_bt {
*/
void WORDY_PRINT(const char * format, ...) noexcept;
- #ifdef PERF_PRINT_ON
- #define PERF_TS_T0() const uint64_t _t0 = direct_bt::getCurrentMilliseconds()
+ #define PERF_TS_T0_BASE() const uint64_t _t0 = direct_bt::getCurrentMilliseconds()
- #define PERF_TS_TD(m) { const uint64_t _td = direct_bt::getCurrentMilliseconds() - _t0; \
- fprintf(stderr, "[%'9" PRIu64 "] %s done in %d ms,\n", direct_bt::DBTEnv::getElapsedMillisecond(), (m), (int)_td); }
+ #define PERF_TS_TD_BASE(m) { const uint64_t _td = direct_bt::getCurrentMilliseconds() - _t0; \
+ fprintf(stderr, "[%'9" PRIu64 "] PERF %s done in %d ms,\n", direct_bt::DBTEnv::getElapsedMillisecond(), (m), (int)_td); }
+ #ifdef PERF_PRINT_ON
+ #define PERF_TS_T0() PERF_TS_T0_BASE()
+ #define PERF_TS_TD(m) PERF_TS_TD_BASE(m)
#else
#define PERF_TS_T0()
#define PERF_TS_TD(m)
#endif
+ #ifdef PERF2_PRINT_ON
+ #define PERF2_TS_T0() PERF_TS_T0_BASE()
+ #define PERF2_TS_TD(m) PERF_TS_TD_BASE(m)
+ #else
+ #define PERF2_TS_T0()
+ #define PERF2_TS_TD(m)
+ #endif
+ #ifdef PERF3_PRINT_ON
+ #define PERF3_TS_T0() PERF_TS_T0_BASE()
+ #define PERF3_TS_TD(m) PERF_TS_TD_BASE(m)
+ #else
+ #define PERF3_TS_T0()
+ #define PERF3_TS_TD(m)
+ #endif
/** Use for unconditional ::abort() call with given messages, prefix '[elapsed_time] ABORT @ file:line: '. Function also appends last errno and strerror(errno). */
void ABORT_impl(const char *file, const int line, const char * format, ...) noexcept;
diff --git a/src/direct_bt/DBTManager.cpp b/src/direct_bt/DBTManager.cpp
index 3170f7cd..8ecfd46a 100644
--- a/src/direct_bt/DBTManager.cpp
+++ b/src/direct_bt/DBTManager.cpp
@@ -33,6 +33,8 @@
#include <algorithm>
// #define PERF_PRINT_ON 1
+// PERF3_PRINT_ON for close
+// #define PERF3_PRINT_ON 1
#include <dbt_debug.hpp>
#include "BTIoctl.hpp"
@@ -79,7 +81,7 @@ std::mutex DBTManager::mtx_singleton;
void DBTManager::mgmtReaderThreadImpl() noexcept {
{
- const std::lock_guard<std::mutex> lock(mtx_mgmtReaderInit); // RAII-style acquire and relinquish via destructor
+ const std::lock_guard<std::mutex> lock(mtx_mgmtReaderLifecycle); // RAII-style acquire and relinquish via destructor
mgmtReaderShallStop = false;
mgmtReaderRunning = true;
DBG_PRINT("DBTManager::reader: Started");
@@ -121,10 +123,15 @@ void DBTManager::mgmtReaderThreadImpl() noexcept {
ERR_PRINT("DBTManager::reader: HCIComm read error");
}
}
+ {
+ const std::lock_guard<std::mutex> lock(mtx_mgmtReaderLifecycle); // RAII-style acquire and relinquish via destructor
+ WORDY_PRINT("DBTManager::reader: Ended. Ring has %d entries flushed", mgmtEventRing.getSize());
+ mgmtEventRing.clear();
+ mgmtReaderRunning = false;
+ cv_mgmtReaderInit.notify_all();
+ }
+
- WORDY_PRINT("DBTManager::reader: Ended. Ring has %d entries flushed", mgmtEventRing.getSize());
- mgmtReaderRunning = false;
- mgmtEventRing.clear();
}
void DBTManager::sendMgmtEvent(std::shared_ptr<MgmtEvent> event) noexcept {
@@ -301,10 +308,11 @@ DBTManager::DBTManager(const BTMode _defaultBTMode) noexcept
: env(MgmtEnv::get()),
defaultBTMode(BTMode::NONE != _defaultBTMode ? _defaultBTMode : env.DEFAULT_BTMODE),
rbuffer(ClientMaxMTU), comm(HCI_DEV_NONE, HCI_CHANNEL_CONTROL),
- mgmtEventRing(env.MGMT_EVT_RING_CAPACITY), mgmtReaderRunning(false), mgmtReaderShallStop(false)
+ mgmtEventRing(env.MGMT_EVT_RING_CAPACITY), mgmtReaderRunning(false), mgmtReaderShallStop(false),
+ allowClose( comm.isOpen() )
{
WORDY_PRINT("DBTManager.ctor: BTMode %s, pid %d", getBTModeString(defaultBTMode).c_str(), DBTManager::pidSelf);
- if( !comm.isOpen() ) {
+ if( !allowClose ) {
ERR_PRINT("DBTManager::open: Could not open mgmt control channel");
return;
}
@@ -320,8 +328,13 @@ DBTManager::DBTManager(const BTMode _defaultBTMode) noexcept
}
}
{
- std::unique_lock<std::mutex> lock(mtx_mgmtReaderInit); // RAII-style acquire and relinquish via destructor
- mgmtReaderThread = std::thread(&DBTManager::mgmtReaderThreadImpl, this);
+ std::unique_lock<std::mutex> lock(mtx_mgmtReaderLifecycle); // RAII-style acquire and relinquish via destructor
+ std::thread mgmtReaderThread = std::thread(&DBTManager::mgmtReaderThreadImpl, this);
+ mgmtReaderThreadId = mgmtReaderThread.native_handle();
+ // Avoid 'terminate called without an active exception'
+ // as hciReaderThreadImpl may end due to I/O errors.
+ mgmtReaderThread.detach();
+
while( false == mgmtReaderRunning ) {
cv_mgmtReaderInit.wait(lock);
}
@@ -448,10 +461,22 @@ fail:
}
void DBTManager::close() noexcept {
- DBG_PRINT("DBTManager::close: Start");
+ // Avoid disconnect re-entry -> potential deadlock
+ bool expConn = true; // C++11, exp as value since C++20
+ if( !allowClose.compare_exchange_strong(expConn, false) ) {
+ // not open
+ DBG_PRINT("DBTManager::close: Not open");
+ whitelist.clear();
+ clearAllMgmtEventCallbacks();
+ adapterInfos.clear();
+ comm.close();
+ return;
+ }
+ PERF3_TS_T0();
+ const std::lock_guard<std::recursive_mutex> lock(mtx_sendReply); // RAII-style acquire and relinquish via destructor
+ DBG_PRINT("DBTManager::close: Start");
removeAllDevicesFromWhitelist();
-
clearAllMgmtEventCallbacks();
for (auto it = adapterInfos.begin(); it != adapterInfos.end(); it++) {
@@ -459,19 +484,35 @@ void DBTManager::close() noexcept {
}
adapterInfos.clear();
- if( mgmtReaderRunning && mgmtReaderThread.joinable() ) {
- mgmtReaderShallStop = true;
- pthread_t tid = mgmtReaderThread.native_handle();
- pthread_kill(tid, SIGALRM);
- }
+ // Interrupt DBTManager's HCIComm::read(..), avoiding prolonged hang
+ // and pull all underlying hci read operations!
comm.close();
- if( mgmtReaderRunning && mgmtReaderThread.joinable() ) {
- // still running ..
- DBG_PRINT("DBTManager::close: join mgmtReaderThread");
- mgmtReaderThread.join();
+ PERF3_TS_TD("DBTManager::close.1");
+ {
+ std::unique_lock<std::mutex> lockReader(mtx_mgmtReaderLifecycle); // RAII-style acquire and relinquish via destructor
+ const pthread_t tid_self = pthread_self();
+ const pthread_t tid_reader = mgmtReaderThreadId;
+ mgmtReaderThreadId = 0;
+ const bool is_reader = tid_reader == tid_self;
+ DBG_PRINT("DBTManager::close: mgmtReader[running %d, shallStop %d, isReader %d, tid %p)",
+ mgmtReaderRunning.load(), mgmtReaderShallStop.load(), is_reader, (void*)tid_reader);
+ if( mgmtReaderRunning ) {
+ mgmtReaderShallStop = true;
+ if( !is_reader && 0 != tid_reader ) {
+ int kerr;
+ if( 0 != ( kerr = pthread_kill(tid_reader, SIGALRM) ) ) {
+ ERR_PRINT("DBTManager::close: pthread_kill %p FAILED: %d", (void*)tid_reader, kerr);
+ }
+ }
+ // Ensure the reader thread has ended, no runaway-thread using *this instance after destruction
+ while( true == mgmtReaderRunning ) {
+ cv_mgmtReaderInit.wait(lockReader);
+ }
+ }
}
- mgmtReaderThread = std::thread(); // empty
+ PERF3_TS_TD("DBTManager::close.2");
+
{
struct sigaction sa_setup;
bzero(&sa_setup, sizeof(sa_setup));
@@ -482,6 +523,8 @@ void DBTManager::close() noexcept {
ERR_PRINT("DBTManager.sigaction: Resetting sighandler");
}
}
+
+ PERF3_TS_TD("DBTManager::close.X");
DBG_PRINT("DBTManager::close: End");
}
diff --git a/src/direct_bt/GATTHandler.cpp b/src/direct_bt/GATTHandler.cpp
index 3414765d..566b0cf9 100644
--- a/src/direct_bt/GATTHandler.cpp
+++ b/src/direct_bt/GATTHandler.cpp
@@ -40,18 +40,12 @@ extern "C" {
}
// #define PERF_PRINT_ON 1
+// PERF2_PRINT_ON for read/write single values
// #define PERF2_PRINT_ON 1
+// PERF3_PRINT_ON for disconnect
+// #define PERF3_PRINT_ON 1
#include <dbt_debug.hpp>
-// PERF2_PRINT_ON for read/write single values
-#ifdef PERF2_PRINT_ON
- #define PERF2_TS_T0() PERF_TS_T0()
- #define PERF2_TS_TD(m) PERF_TS_TD(m)
-#else
- #define PERF2_TS_T0()
- #define PERF2_TS_TD(m)
-#endif
-
#include "BasicAlgos.hpp"
#include "L2CAPIoctl.hpp"
@@ -189,17 +183,17 @@ bool GATTHandler::getSendIndicationConfirmation() noexcept {
void GATTHandler::l2capReaderThreadImpl() {
{
- const std::lock_guard<std::mutex> lock(mtx_l2capReaderInit); // RAII-style acquire and relinquish via destructor
+ const std::lock_guard<std::mutex> lock(mtx_l2capReaderLifecycle); // RAII-style acquire and relinquish via destructor
l2capReaderShallStop = false;
l2capReaderRunning = true;
- DBG_PRINT("l2capReaderThreadImpl Started");
+ DBG_PRINT("GATTHandler::reader Started");
cv_l2capReaderInit.notify_all();
}
while( !l2capReaderShallStop ) {
int len;
if( !validateConnected() ) {
- ERR_PRINT("GATTHandler::l2capReaderThread: Invalid IO state -> Stop");
+ ERR_PRINT("GATTHandler::reader: Invalid IO state -> Stop");
l2capReaderShallStop = true;
break;
}
@@ -211,7 +205,7 @@ void GATTHandler::l2capReaderThreadImpl() {
if( AttPDUMsg::Opcode::ATT_HANDLE_VALUE_NTF == opc ) {
const AttHandleValueRcv * a = static_cast<const AttHandleValueRcv*>(attPDU.get());
- COND_PRINT(env.DEBUG_DATA, "GATTHandler: NTF: %s, listener %zd", a->toString().c_str(), characteristicListenerList.size());
+ COND_PRINT(env.DEBUG_DATA, "GATTHandler::reader: NTF: %s, listener %zd", a->toString().c_str(), characteristicListenerList.size());
GATTCharacteristicRef decl = findCharacterisicsByValueHandle(a->getHandle());
const std::shared_ptr<TROOctets> data(new POctets(a->getValue()));
const uint64_t timestamp = a->ts_creation;
@@ -230,7 +224,7 @@ void GATTHandler::l2capReaderThreadImpl() {
});
} else if( AttPDUMsg::Opcode::ATT_HANDLE_VALUE_IND == opc ) {
const AttHandleValueRcv * a = static_cast<const AttHandleValueRcv*>(attPDU.get());
- COND_PRINT(env.DEBUG_DATA, "GATTHandler: IND: %s, sendIndicationConfirmation %d, listener %zd", a->toString().c_str(), sendIndicationConfirmation, characteristicListenerList.size());
+ COND_PRINT(env.DEBUG_DATA, "GATTHandler::reader: IND: %s, sendIndicationConfirmation %d, listener %zd", a->toString().c_str(), sendIndicationConfirmation, characteristicListenerList.size());
bool cfmSent = false;
if( sendIndicationConfirmation ) {
AttHandleValueCfm cfm;
@@ -255,20 +249,23 @@ void GATTHandler::l2capReaderThreadImpl() {
});
} else if( AttPDUMsg::Opcode::ATT_MULTIPLE_HANDLE_VALUE_NTF == opc ) {
// FIXME TODO ..
- ERR_PRINT("GATTHandler: MULTI-NTF not implemented: %s", attPDU->toString().c_str());
+ ERR_PRINT("GATTHandler::reader: MULTI-NTF not implemented: %s", attPDU->toString().c_str());
} else {
attPDURing.putBlocking( attPDU );
}
} else if( ETIMEDOUT != errno && !l2capReaderShallStop ) { // expected exits
- IRQ_PRINT("GATTHandler::l2capReaderThread: l2cap read error -> Stop; l2cap.read %d", len);
+ IRQ_PRINT("GATTHandler::reader: l2cap read error -> Stop; l2cap.read %d", len);
l2capReaderShallStop = true;
has_ioerror = true;
}
}
-
- WORDY_PRINT("l2capReaderThreadImpl Ended. Ring has %d entries flushed", attPDURing.getSize());
- l2capReaderRunning = false;
- attPDURing.clear();
+ {
+ const std::lock_guard<std::mutex> lock(mtx_l2capReaderLifecycle); // RAII-style acquire and relinquish via destructor
+ WORDY_PRINT("GATTHandler::reader: Ended. Ring has %d entries flushed", attPDURing.getSize());
+ attPDURing.clear();
+ l2capReaderRunning = false;
+ cv_l2capReaderInit.notify_all();
+ }
disconnect(true /* disconnectDevice */, has_ioerror);
}
@@ -294,7 +291,7 @@ GATTHandler::GATTHandler(const std::shared_ptr<DBTDevice> &device) noexcept
* as we only can install one handler.
*/
{
- std::unique_lock<std::mutex> lock(mtx_l2capReaderInit); // RAII-style acquire and relinquish via destructor
+ std::unique_lock<std::mutex> lock(mtx_l2capReaderLifecycle); // RAII-style acquire and relinquish via destructor
std::thread l2capReaderThread = std::thread(&GATTHandler::l2capReaderThreadImpl, this);
l2capReaderThreadId = l2capReaderThread.native_handle();
@@ -333,7 +330,8 @@ GATTHandler::~GATTHandler() noexcept {
}
bool GATTHandler::disconnect(const bool disconnectDevice, const bool ioErrorCause) noexcept {
- // Interrupt GATT's L2CAP ::connect(..), avoiding prolonged hang
+ PERF3_TS_T0();
+ // Interrupt GATT's L2CAP::connect(..) and L2CAP::read(..), avoiding prolonged hang
// and pull all underlying l2cap read operations!
l2cap.disconnect();
@@ -346,13 +344,16 @@ bool GATTHandler::disconnect(const bool disconnectDevice, const bool ioErrorCaus
characteristicListenerList.clear();
return false;
}
- {
- // Lock to avoid other threads using instance while disconnecting
- const std::lock_guard<std::recursive_mutex> lock(mtx_command); // RAII-style acquire and relinquish via destructor
+ // Lock to avoid other threads using instance while disconnecting
+ const std::lock_guard<std::recursive_mutex> lock(mtx_command); // RAII-style acquire and relinquish via destructor
+ DBG_PRINT("GATTHandler::disconnect: Start: disconnectDevice %d, ioErrorCause %d: GattHandler[%s], l2cap[%s]: %s",
+ disconnectDevice, ioErrorCause, getStateString().c_str(), l2cap.getStateString().c_str(), deviceString.c_str());
+ removeAllCharacteristicListener();
+ PERF3_TS_TD("GATTHandler::disconnect.1");
+ {
+ std::unique_lock<std::mutex> lockReader(mtx_l2capReaderLifecycle); // RAII-style acquire and relinquish via destructor
has_ioerror = false;
- DBG_PRINT("GATTHandler::disconnect: Start: disconnectDevice %d, ioErrorCause %d: GattHandler[%s], l2cap[%s]: %s",
- disconnectDevice, ioErrorCause, getStateString().c_str(), l2cap.getStateString().c_str(), deviceString.c_str());
const pthread_t tid_self = pthread_self();
const pthread_t tid_l2capReader = l2capReaderThreadId;
@@ -368,9 +369,13 @@ bool GATTHandler::disconnect(const bool disconnectDevice, const bool ioErrorCaus
ERR_PRINT("GATTHandler::disconnect: pthread_kill %p FAILED: %d", (void*)tid_l2capReader, kerr);
}
}
+ // Ensure the reader thread has ended, no runaway-thread using *this instance after destruction
+ while( true == l2capReaderRunning ) {
+ cv_l2capReaderInit.wait(lockReader);
+ }
}
- removeAllCharacteristicListener();
}
+ PERF3_TS_TD("GATTHandler::disconnect.2");
if( disconnectDevice ) {
std::shared_ptr<DBTDevice> device = getDeviceUnchecked();
@@ -384,6 +389,7 @@ bool GATTHandler::disconnect(const bool disconnectDevice, const bool ioErrorCaus
}
}
+ PERF3_TS_TD("GATTHandler::disconnect.X");
DBG_PRINT("GATTHandler::disconnect: End: %s", deviceString.c_str());
return true;
}
diff --git a/src/direct_bt/HCIComm.cpp b/src/direct_bt/HCIComm.cpp
index 2ab24734..ffcfc97c 100644
--- a/src/direct_bt/HCIComm.cpp
+++ b/src/direct_bt/HCIComm.cpp
@@ -32,7 +32,7 @@
#include <algorithm>
-// #define VERBOSE_ON 1
+// #define PERF_PRINT_ON 1
#include <dbt_debug.hpp>
#include "HCIComm.hpp"
@@ -95,13 +95,44 @@ int HCIComm::hci_close_dev(int dd) noexcept
return ::close(dd);
}
+// *************************************************
+// *************************************************
+// *************************************************
+
+HCIComm::HCIComm(const uint16_t _dev_id, const uint16_t _channel) noexcept
+: dev_id( _dev_id ), channel( _channel ),
+ socket_descriptor( hci_open_dev(_dev_id, _channel) ), interrupt_flag(false), tid_read(0)
+{
+}
+
void HCIComm::close() noexcept {
const std::lock_guard<std::recursive_mutex> lock(mtx_write); // RAII-style acquire and relinquish via destructor
if( 0 > socket_descriptor ) {
+ DBG_PRINT("HCIComm::close: Not opened: dd %d", socket_descriptor.load());
return;
}
+ DBG_PRINT("HCIComm::close: Start: dd %d", socket_descriptor.load());
+ PERF_TS_T0();
+ // interrupt ::read(..) and , avoiding prolonged hang
+ interrupt_flag = true;
+ {
+ pthread_t _tid_read = tid_read;
+ tid_read = 0;
+ if( 0 != _tid_read ) {
+ pthread_t tid_self = pthread_self();
+ if( tid_self != _tid_read ) {
+ int kerr;
+ if( 0 != ( kerr = pthread_kill(_tid_read, SIGALRM) ) ) {
+ ERR_PRINT("HCIComm::close: pthread_kill read %p FAILED: %d", (void*)_tid_read, kerr);
+ }
+ }
+ }
+ }
hci_close_dev(socket_descriptor);
socket_descriptor = -1;
+ interrupt_flag = false;
+ PERF_TS_TD("HCIComm::close");
+ DBG_PRINT("HCIComm::close: End: dd %d", socket_descriptor.load());
}
int HCIComm::read(uint8_t* buffer, const int capacity, const int32_t timeoutMS) noexcept {
@@ -118,18 +149,8 @@ int HCIComm::read(uint8_t* buffer, const int capacity, const int32_t timeoutMS)
int n;
p.fd = socket_descriptor; p.events = POLLIN;
-#if 0
- sigset_t sigmask;
- sigemptyset(&sigmask);
- // sigaddset(&sigmask, SIGALRM);
- struct timespec timeout_ts;
- timeout_ts.tv_sec=0;
- timeout_ts.tv_nsec=(long)timeoutMS*1000000L;
- while ((n = ppoll(&p, 1, &timeout_ts, &sigmask)) < 0) {
-#else
- while ((n = poll(&p, 1, timeoutMS)) < 0) {
-#endif
- if (errno == EAGAIN || errno == EINTR ) {
+ while ( !interrupt_flag && (n = poll(&p, 1, timeoutMS)) < 0 ) {
+ if ( !interrupt_flag && ( errno == EAGAIN || errno == EINTR ) ) {
// cont temp unavail or interruption
continue;
}
diff --git a/src/direct_bt/HCIHandler.cpp b/src/direct_bt/HCIHandler.cpp
index 7664fef1..5ee14c5d 100644
--- a/src/direct_bt/HCIHandler.cpp
+++ b/src/direct_bt/HCIHandler.cpp
@@ -241,7 +241,7 @@ std::shared_ptr<MgmtEvent> HCIHandler::translate(std::shared_ptr<HCIEvent> ev) n
void HCIHandler::hciReaderThreadImpl() noexcept {
{
- const std::lock_guard<std::mutex> lock(mtx_hciReaderInit); // RAII-style acquire and relinquish via destructor
+ const std::lock_guard<std::mutex> lock(mtx_hciReaderLifecycle); // RAII-style acquire and relinquish via destructor
hciReaderShallStop = false;
hciReaderRunning = true;
DBG_PRINT("HCIHandler::reader: Started");
@@ -291,12 +291,9 @@ void HCIHandler::hciReaderThreadImpl() noexcept {
} else if( event->isMetaEvent(HCIMetaEventType::LE_ADVERTISING_REPORT) ) {
// issue callbacks for the translated AD events
std::vector<std::shared_ptr<EInfoReport>> eirlist = EInfoReport::read_ad_reports(event->getParam(), event->getParamSize());
- int i=0;
- for_each_idx(eirlist, [&](std::shared_ptr<EInfoReport> &eir) {
+ for_each_idx(eirlist, [&](std::shared_ptr<EInfoReport> & eir) {
// COND_PRINT(env.DEBUG_EVENT, "HCIHandler-IO RECV (AD EIR) %s", eir->toString().c_str());
- std::shared_ptr<MgmtEvent> mevent( new MgmtEvtDeviceFound(dev_id, eir) );
- sendMgmtEvent( mevent );
- i++;
+ sendMgmtEvent( std::shared_ptr<MgmtEvent>( new MgmtEvtDeviceFound(dev_id, eir) ) );
});
} else {
// issue a callback for the translated event
@@ -312,9 +309,13 @@ void HCIHandler::hciReaderThreadImpl() noexcept {
ERR_PRINT("HCIHandler::reader: HCIComm read error");
}
}
- WORDY_PRINT("HCIHandler::reader: Ended. Ring has %d entries flushed", hciEventRing.getSize());
- hciReaderRunning = false;
- hciEventRing.clear();
+ {
+ const std::lock_guard<std::mutex> lock(mtx_hciReaderLifecycle); // RAII-style acquire and relinquish via destructor
+ WORDY_PRINT("HCIHandler::reader: Ended. Ring has %d entries flushed", hciEventRing.getSize());
+ hciEventRing.clear();
+ hciReaderRunning = false;
+ cv_hciReaderInit.notify_all();
+ }
}
void HCIHandler::sendMgmtEvent(std::shared_ptr<MgmtEvent> event) noexcept {
@@ -424,16 +425,17 @@ HCIHandler::HCIHandler(const BTMode btMode, const uint16_t dev_id) noexcept
: env(HCIEnv::get()),
btMode(btMode), dev_id(dev_id), rbuffer(HCI_MAX_MTU),
comm(dev_id, HCI_CHANNEL_RAW),
- hciEventRing(env.HCI_EVT_RING_CAPACITY), hciReaderRunning(false), hciReaderShallStop(false)
+ hciEventRing(env.HCI_EVT_RING_CAPACITY), hciReaderRunning(false), hciReaderShallStop(false),
+ allowClose( comm.isOpen() )
{
WORDY_PRINT("HCIHandler.ctor: pid %d", HCIHandler::pidSelf);
- if( !comm.isOpen() ) {
+ if( !allowClose ) {
ERR_PRINT("HCIHandler::ctor: Could not open hci control channel");
return;
}
{
- std::unique_lock<std::mutex> lock(mtx_hciReaderInit); // RAII-style acquire and relinquish via destructor
+ std::unique_lock<std::mutex> lock(mtx_hciReaderLifecycle); // RAII-style acquire and relinquish via destructor
std::thread hciReaderThread = std::thread(&HCIHandler::hciReaderThreadImpl, this);
hciReaderThreadId = hciReaderThread.native_handle();
@@ -525,27 +527,48 @@ fail:
}
void HCIHandler::close() noexcept {
+ // Avoid disconnect re-entry -> potential deadlock
+ bool expConn = true; // C++11, exp as value since C++20
+ if( !allowClose.compare_exchange_strong(expConn, false) ) {
+ // not open
+ DBG_PRINT("HCIHandler::close: Not open");
+ clearAllMgmtEventCallbacks();
+ comm.close();
+ return;
+ }
+ PERF_TS_T0();
const std::lock_guard<std::recursive_mutex> lock(mtx); // RAII-style acquire and relinquish via destructor
DBG_PRINT("HCIHandler::close: Start");
-
clearAllMgmtEventCallbacks();
- const pthread_t tid_self = pthread_self();
- const pthread_t tid_reader = hciReaderThreadId;
- hciReaderThreadId = 0;
- const bool is_reader = tid_reader == tid_self;
- DBG_PRINT("HCIHandler.disconnect: Start hciReader[running %d, shallStop %d, isReader %d, tid %p)",
- hciReaderRunning.load(), hciReaderShallStop.load(), is_reader, (void*)tid_reader);
- if( hciReaderRunning ) {
- hciReaderShallStop = true;
- if( !is_reader && 0 != tid_reader ) {
- int kerr;
- if( 0 != ( kerr = pthread_kill(tid_reader, SIGALRM) ) ) {
- ERR_PRINT("HCIHandler::disconnect: pthread_kill %p FAILED: %d", (void*)tid_reader, kerr);
+ // Interrupt HCIHandler's HCIComm::read(..), avoiding prolonged hang
+ // and pull all underlying hci read operations!
+ comm.close();
+
+ PERF_TS_TD("HCIHandler::close.1");
+ {
+ std::unique_lock<std::mutex> lockReader(mtx_hciReaderLifecycle); // RAII-style acquire and relinquish via destructor
+ const pthread_t tid_self = pthread_self();
+ const pthread_t tid_reader = hciReaderThreadId;
+ hciReaderThreadId = 0;
+ const bool is_reader = tid_reader == tid_self;
+ DBG_PRINT("HCIHandler::close: hciReader[running %d, shallStop %d, isReader %d, tid %p)",
+ hciReaderRunning.load(), hciReaderShallStop.load(), is_reader, (void*)tid_reader);
+ if( hciReaderRunning ) {
+ hciReaderShallStop = true;
+ if( !is_reader && 0 != tid_reader ) {
+ int kerr;
+ if( 0 != ( kerr = pthread_kill(tid_reader, SIGALRM) ) ) {
+ ERR_PRINT("HCIHandler::close: pthread_kill %p FAILED: %d", (void*)tid_reader, kerr);
+ }
+ }
+ // Ensure the reader thread has ended, no runaway-thread using *this instance after destruction
+ while( true == hciReaderRunning ) {
+ cv_hciReaderInit.wait(lockReader);
}
}
}
- comm.close();
+ PERF_TS_TD("HCIHandler::close.X");
DBG_PRINT("HCIHandler::close: End");
}
diff --git a/src/direct_bt/L2CAPComm.cpp b/src/direct_bt/L2CAPComm.cpp
index 7d05c0f3..af81807a 100644
--- a/src/direct_bt/L2CAPComm.cpp
+++ b/src/direct_bt/L2CAPComm.cpp
@@ -32,6 +32,9 @@
#include <algorithm>
+// #define PERF_PRINT_ON 1
+#include <dbt_debug.hpp>
+
#include "BTIoctl.hpp"
#include "HCIIoctl.hpp"
#include "L2CAPIoctl.hpp"
@@ -47,8 +50,6 @@ extern "C" {
#include <signal.h>
}
-#include <dbt_debug.hpp>
-
using namespace direct_bt;
L2CAPEnv::L2CAPEnv() noexcept
@@ -108,7 +109,7 @@ L2CAPComm::L2CAPComm(std::shared_ptr<DBTDevice> device, const uint16_t psm, cons
: env(L2CAPEnv::get()),
device(device), deviceString(device->getAddressString()), psm(psm), cid(cid),
socket_descriptor( l2cap_open_dev(device->getAdapter().getAddress(), psm, cid, true /* pubaddrAdptr */) ),
- is_connected(true), has_ioerror(false), interrupt_flag(false), tid_connect(0)
+ is_connected(true), has_ioerror(false), interrupt_flag(false), tid_connect(0), tid_read(0)
{
/** BT Core Spec v5.2: Vol 3, Part A: L2CAP_CONNECTION_REQ */
sockaddr_l2 req;
@@ -179,17 +180,29 @@ bool L2CAPComm::disconnect() noexcept {
has_ioerror = false;
DBG_PRINT("L2CAPComm::disconnect: Start: %s, dd %d, %s, psm %u, cid %u, pubDevice %d",
getStateString().c_str(), socket_descriptor.load(), deviceString.c_str(), psm, cid, true);
- interrupt_flag = true;
+ PERF_TS_T0();
- // interrupt L2CAP ::connect(..), avoiding prolonged hang
- pthread_t _tid_connect = tid_connect;
- tid_connect = 0;
- if( 0 != _tid_connect ) {
+ interrupt_flag = true;
+ {
pthread_t tid_self = pthread_self();
- if( tid_self != _tid_connect ) {
+ pthread_t _tid_connect = tid_connect;
+ pthread_t _tid_read = tid_read;
+ tid_read = 0;
+ tid_connect = 0;
+
+ // interrupt read(..) and , avoiding prolonged hang
+ if( 0 != _tid_read && tid_self != _tid_read ) {
+ int kerr;
+ if( 0 != ( kerr = pthread_kill(_tid_read, SIGALRM) ) ) {
+ ERR_PRINT("L2CAPComm::disconnect: pthread_kill read %p FAILED: %d", (void*)_tid_read, kerr);
+ }
+ }
+ // interrupt connect(..) and , avoiding prolonged hang
+ interrupt_flag = true;
+ if( 0 != _tid_connect && _tid_read != _tid_connect && tid_self != _tid_connect ) {
int kerr;
if( 0 != ( kerr = pthread_kill(_tid_connect, SIGALRM) ) ) {
- ERR_PRINT("L2CAP::disconnect: pthread_kill %p FAILED: %d", (void*)_tid_connect, kerr);
+ ERR_PRINT("L2CAPComm::disconnect: pthread_kill connect %p FAILED: %d", (void*)_tid_connect, kerr);
}
}
}
@@ -197,6 +210,7 @@ bool L2CAPComm::disconnect() noexcept {
l2cap_close_dev(socket_descriptor);
socket_descriptor = -1;
interrupt_flag = false;
+ PERF_TS_TD("L2CAPComm::disconnect");
DBG_PRINT("L2CAPComm::disconnect: End: dd %d", socket_descriptor.load());
return true;
}
@@ -206,6 +220,8 @@ int L2CAPComm::read(uint8_t* buffer, const int capacity) {
int len = 0;
int err_res = 0;
+ tid_read = pthread_self(); // temporary safe tid to allow interruption
+
if( 0 > socket_descriptor || 0 > capacity ) {
err_res = -1; // invalid socket_descriptor or capacity
goto errout;
@@ -244,9 +260,11 @@ int L2CAPComm::read(uint8_t* buffer, const int capacity) {
}
done:
+ tid_read = 0;
return len;
errout:
+ tid_read = 0;
if( errno != ETIMEDOUT ) {
has_ioerror = true;
if( is_connected ) {