You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by "mickeyl (via GitHub)" <gi...@apache.org> on 2024/03/29 15:18:32 UTC
[I] L2CAP COC: Preventing a race condition [mynewt-nimble]
mickeyl opened a new issue, #1741:
URL: https://github.com/apache/mynewt-nimble/issues/1741
I'm currently debugging a race condition. There are two threads involved. My custom one is trying to `ble_l2cap_send`, the other one (`nimble_host`) is handling the `disconnect` event.
Here are the corresponding tracebacks. The occurs in the thread `MCP-Receiver`:
```
ble_hs_hci_acl_tx(struct ble_hs_conn * conn, struct os_mbuf ** om) (/home/mickey/Dokumente/late/ecos/esp-idf/components/bt/host/nimble/nimble/nimble/host/src/ble_hs_hci.c:617)
ble_l2cap_tx(struct ble_hs_conn * conn, struct ble_l2cap_chan * chan, struct os_mbuf * txom) (/home/mickey/Dokumente/late/ecos/esp-idf/components/bt/host/nimble/nimble/nimble/host/src/ble_l2cap.c:467)
ble_l2cap_coc_continue_tx(struct ble_l2cap_chan * chan) (/home/mickey/Dokumente/late/ecos/esp-idf/components/bt/host/nimble/nimble/nimble/host/src/ble_l2cap_coc.c:493)
ble_l2cap_coc_send(struct ble_l2cap_chan * chan, struct os_mbuf * sdu_tx) (/home/mickey/Dokumente/late/ecos/esp-idf/components/bt/host/nimble/nimble/nimble/host/src/ble_l2cap_coc.c:644)
ble_l2cap_send(struct ble_l2cap_chan * chan, struct os_mbuf * sdu) (/home/mickey/Dokumente/late/ecos/esp-idf/components/bt/host/nimble/nimble/nimble/host/src/ble_l2cap.c:227)
NimBLEL2CAPService::write(NimBLEL2CAPService * const this, const std::vector<unsigned char, std::allocator<unsigned char> > & bytes) (/home/mickey/Dokumente/late/ecos/esp-nimble-cpp/src/NimBLEL2CAPService.cpp:77)
operator()(std::vector<unsigned char, std::allocator<unsigned char> > & data, const struct {...} * const __closure) (/home/mickey/Dokumente/late/ecos/firmware/main/Bluetooth/L2CAPServiceCallbacks.cpp:22)
std::__invoke_impl<void, L2CAPServiceCallbacks::onConnect(NimBLEL2CAPService*)::<lambda(std::vector<unsigned char>&)>&, std::vector<unsigned char, std::allocator<unsigned char> >&>(struct {...} & __f) (/home/mickey/.espressif/tools/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/xtensa-esp-elf/include/c++/13.2.0/bits/invoke.h:61)
std::__invoke_r<void, L2CAPServiceCallbacks::onConnect(NimBLEL2CAPService*)::<lambda(std::vector<unsigned char>&)>&, std::vector<unsigned char, std::allocator<unsigned char> >&>(struct {...} & __fn) (/home/mickey/.espressif/tools/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/xtensa-esp-elf/include/c++/13.2.0/bits/invoke.h:111)
std::_Function_handler<void(std::vector<unsigned char, std::allocator<unsigned char> >&), L2CAPServiceCallbacks::onConnect(NimBLEL2CAPService*)::<lambda(std::vector<unsigned char, std::allocator<unsigned char> >&)> >::_M_invoke(const std::_Any_data &, std::vector<unsigned char, std::allocator<unsigned char> > &)(const std::_Any_data & __functor, __args#0) (/home/mickey/.espressif/tools/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/xtensa-esp-elf/include/c++/13.2.0/bits/std_function.h:290)
std::function<void (std::vector<unsigned char, std::allocator<unsigned char> >&)>::operator()(std::vector<unsigned char, std::allocator<unsigned char> >&) const(const std::function<void(std::vector<unsigned char, std::allocator<unsigned char> >&)> * const this, __args#0) (/home/mickey/.espressif/tools/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/xtensa-esp-elf/include/c++/13.2.0/bits/std_function.h:591)
ProtocolMachine::writePDU(ProtocolMachine * const this, CANyonero::PDU & pdu) (/home/mickey/Dokumente/late/ecos/firmware/main/ProtocolMachine/ProtocolMachine.cpp:192)
ProtocolMachine::writePDUForIncomingData(ProtocolMachine * const this, const int channelNumber, const uint32_t id, const uint8_t extension, const std::vector<unsigned char, std::allocator<unsigned char> > & bytes) (/home/mickey/Dokumente/late/ecos/firmware/main/ProtocolMachine/ProtocolMachine.cpp:199)
CANChannelManager::handleIncomingFrame(CANChannelManager * const this, CANMessage & frame) (/home/mickey/Dokumente/late/ecos/firmware/main/CAN/CANChannelManager.cpp:116)
CANController::run(CANController * const this, void * data) (/home/mickey/Dokumente/late/ecos/firmware/main/CAN/CANController.cpp:76)
Task::runTask(void * pTaskInstance) (/home/mickey/Dokumente/late/ecos/ESPenlaub/FreeRTOS/Task.hpp:175)
vPortTaskWrapper(TaskFunction_t pxCode, void * pvParameters) (/home/mickey/Dokumente/late/ecos/esp-idf/components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c:134)
```
While the channel is being shutdown, the `conn` is suddently NULL. Since the NimBLE code does not check for this case, it crashes.
The root cause of this is the shutdown being handled by `nimble_host`:
```
[Unknown/Just-In-Time compiled code] (Unknown Source:0)
vPortClearInterruptMaskFromISR() (/home/mickey/Dokumente/late/ecos/esp-idf/components/freertos/FreeRTOS-Kernel/portable/xtensa/include/freertos/portmacro.h:564)
vPortExitCritical(portMUX_TYPE * mux) (/home/mickey/Dokumente/late/ecos/esp-idf/components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c:504)
xQueueSemaphoreTake(QueueHandle_t xQueue, TickType_t xTicksToWait) (/home/mickey/Dokumente/late/ecos/esp-idf/components/freertos/FreeRTOS-Kernel/queue.c:1853)
Task::cancel(Task * const this) (/home/mickey/Dokumente/late/ecos/ESPenlaub/FreeRTOS/Task.hpp:102)
CANController::~CANController(CANController * const this) (/home/mickey/Dokumente/late/ecos/firmware/main/CAN/CANController.cpp:36)
CANController::~CANController(CANController * const this) (/home/mickey/Dokumente/late/ecos/firmware/main/CAN/CANController.cpp:39)
std::default_delete<CANController>::operator()(CANController * __ptr) (/home/mickey/.espressif/tools/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/xtensa-esp-elf/include/c++/13.2.0/bits/unique_ptr.h:93)
std::__uniq_ptr_impl<CANController, std::default_delete<CANController> >::reset(std::__uniq_ptr_impl<CANController, std::default_delete<CANController> >::pointer __p, std::__uniq_ptr_impl<CANController, std::default_delete<CANController> > * const this) (/home/mickey/.espressif/tools/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/xtensa-esp-elf/include/c++/13.2.0/bits/unique_ptr.h:211)
std::unique_ptr<CANController, std::default_delete<CANController> >::reset(std::unique_ptr<CANController, std::default_delete<CANController> >::pointer __p, std::unique_ptr<CANController, std::default_delete<CANController> > * const this) (/home/mickey/.espressif/tools/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/xtensa-esp-elf/include/c++/13.2.0/bits/unique_ptr.h:509)
CANChannelManager::closeCAN(CANChannelManager * const this) (/home/mickey/Dokumente/late/ecos/firmware/main/CAN/CANChannelManager.cpp:143)
CANChannelManager::~CANChannelManager(CANChannelManager * const this) (/home/mickey/Dokumente/late/ecos/firmware/main/CAN/CANChannelManager.cpp:28)
std::default_delete<CANChannelManager>::operator()(const std::default_delete<CANChannelManager> * const this, CANChannelManager * __ptr) (/home/mickey/.espressif/tools/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/xtensa-esp-elf/include/c++/13.2.0/bits/unique_ptr.h:99)
std::unique_ptr<CANChannelManager, std::default_delete<CANChannelManager> >::~unique_ptr(std::unique_ptr<CANChannelManager, std::default_delete<CANChannelManager> > * const this) (/home/mickey/.espressif/tools/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/xtensa-esp-elf/include/c++/13.2.0/tuple:125)
ProtocolMachine::~ProtocolMachine(ProtocolMachine * const this) (/home/mickey/Dokumente/late/ecos/firmware/main/ProtocolMachine/ProtocolMachine.cpp:28)
std::default_delete<ProtocolMachine>::operator()(const std::default_delete<ProtocolMachine> * const this, ProtocolMachine * __ptr) (/home/mickey/.espressif/tools/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/xtensa-esp-elf/include/c++/13.2.0/bits/unique_ptr.h:99)
std::__uniq_ptr_impl<ProtocolMachine, std::default_delete<ProtocolMachine> >::reset(std::__uniq_ptr_impl<ProtocolMachine, std::default_delete<ProtocolMachine> >::pointer __p, std::__uniq_ptr_impl<ProtocolMachine, std::default_delete<ProtocolMachine> > * const this) (/home/mickey/.espressif/tools/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/xtensa-esp-elf/include/c++/13.2.0/bits/unique_ptr.h:211)
std::unique_ptr<ProtocolMachine, std::default_delete<ProtocolMachine> >::reset(std::unique_ptr<ProtocolMachine, std::default_delete<ProtocolMachine> >::pointer __p, std::unique_ptr<ProtocolMachine, std::default_delete<ProtocolMachine> > * const this) (/home/mickey/.espressif/tools/xtensa-esp-elf/esp-13.2.0_20230928/xtensa-esp-elf/xtensa-esp-elf/include/c++/13.2.0/bits/unique_ptr.h:509)
L2CAPServiceCallbacks::onDisconnect(L2CAPServiceCallbacks * const this, NimBLEL2CAPService * pService) (/home/mickey/Dokumente/late/ecos/firmware/main/Bluetooth/L2CAPServiceCallbacks.cpp:73)
NimBLEL2CAPService::handleDisconnectionEvent(NimBLEL2CAPService * const this, ble_l2cap_event * event) (/home/mickey/Dokumente/late/ecos/esp-nimble-cpp/src/NimBLEL2CAPService.cpp:164)
```
I tried to workaround this, but I have no chance of getting to the `conn`, since the `ble_l2cap_chan` is a private structure.
What options do I have?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] L2CAP COC: Preventing a race condition [mynewt-nimble]
Posted by "rymanluk (via GitHub)" <gi...@apache.org>.
rymanluk commented on issue #1741:
URL: https://github.com/apache/mynewt-nimble/issues/1741#issuecomment-2071488039
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] L2CAP COC: Preventing a race condition [mynewt-nimble]
Posted by "mickeyl (via GitHub)" <gi...@apache.org>.
mickeyl commented on issue #1741:
URL: https://github.com/apache/mynewt-nimble/issues/1741#issuecomment-2069380689
@KKopyscinski This patch works very well. Instead of a crash, I get the expected `-ENOTCONN` now.
```
E (9486) NimBLEL2CAPChannel: ble_l2cap_send failed: 7
```
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] L2CAP COC: Preventing a race condition [mynewt-nimble]
Posted by "mickeyl (via GitHub)" <gi...@apache.org>.
mickeyl commented on issue #1741:
URL: https://github.com/apache/mynewt-nimble/issues/1741#issuecomment-2027394324
I looked a bit further and I think a client workaround seems unlikely, since this pretty much happens inside NimBLE.
In `ble_l2cap_coc:492` it says
```c
conn = ble_hs_conn_find_assert(chan->conn_handle);
rc = ble_l2cap_tx(conn, chan, txom);
if (rc) {
/* txom is consumed by l2cap */
txom = NULL;
goto failed;
} else {
tx->credits--;
tx->data_offset += len - sdu_size_offset;
}
```
`ble_hs_conn_find_assert` is supposed to trigger an assertion in debug code, when the `conn_handle` is nowhere to be found in the active connections. For production code, this doesn't happen -- and an assertion surely is too much of a weapon here. I suppose something along the lines of:
```c
conn = ble_hs_conn_find_assert(chan->conn_handle);
if (!conn) {
/* txom is consumed by l2cap */
txom = NULL;
goto failed;
}
rc = ble_l2cap_tx(conn, chan, txom);
if (rc) {
/* txom is consumed by l2cap */
txom = NULL;
goto failed;
} else {
tx->credits--;
tx->data_offset += len - sdu_size_offset;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] L2CAP COC: Preventing a race condition [mynewt-nimble]
Posted by "KKopyscinski (via GitHub)" <gi...@apache.org>.
KKopyscinski commented on issue #1741:
URL: https://github.com/apache/mynewt-nimble/issues/1741#issuecomment-2063022313
Hi, could you please try applying this patch? https://github.com/apache/mynewt-nimble/pull/1766 And please let me know if this works
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] L2CAP COC: Preventing a race condition [mynewt-nimble]
Posted by "rymanluk (via GitHub)" <gi...@apache.org>.
rymanluk closed issue #1741: L2CAP COC: Preventing a race condition
URL: https://github.com/apache/mynewt-nimble/issues/1741
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org