diff options
author | Sven Gothel <[email protected]> | 2020-09-29 18:08:19 +0200 |
---|---|---|
committer | Sven Gothel <[email protected]> | 2020-09-29 18:08:19 +0200 |
commit | 9a0964148e93e14005c8f5425af794b736cdd10d (patch) | |
tree | 4e1ccf41000642f57463fbb98fcd26ebda7153ad | |
parent | 473aaae3080d859c22c640bbb903249f4fe007b7 (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.hpp | 6 | ||||
-rw-r--r-- | api/direct_bt/GATTHandler.hpp | 2 | ||||
-rw-r--r-- | api/direct_bt/HCIComm.hpp | 8 | ||||
-rw-r--r-- | api/direct_bt/HCIHandler.hpp | 4 | ||||
-rw-r--r-- | api/direct_bt/L2CAPComm.hpp | 1 | ||||
-rw-r--r-- | api/direct_bt/dbt_debug.hpp | 24 | ||||
-rw-r--r-- | src/direct_bt/DBTManager.cpp | 83 | ||||
-rw-r--r-- | src/direct_bt/GATTHandler.cpp | 62 | ||||
-rw-r--r-- | src/direct_bt/HCIComm.cpp | 47 | ||||
-rw-r--r-- | src/direct_bt/HCIHandler.cpp | 75 | ||||
-rw-r--r-- | src/direct_bt/L2CAPComm.cpp | 38 |
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 ) { |