we had a deadlock between the mutex in HAL_ChibiOS CAN layer and the
MAVLinkCAN object
the deadlock arose because the low level CAN layer holds the HAL CAN
mutex before it calls up to the MAVLinkCAN frame callback
on the mavlink side, it took the MAVLinkCAN mutex before calling down
to the CAN send layer, this meant it took the 2 mutexes in the
opposite order
the symptoms were:
- failure to boot if we have active MAVCAN during reboot as we
deadlock during the scheduler delay callback
- deadlock during heavy MAVCAN usage
the AP_CANManager::log_text() gets called from debug logging in
AP_DroneCAN. It is a method on a common AP_CANManager object which is
shared by multiple AP_DroneCAN threads.
if two threads call the debug log messages at the same time then we
can end up with _log_pos greater than LOG_BUFFER_SIZE (1024) and
overwrite past the end of the buffer
in the crash_dump we have for this case the next piece of memory was
hal.can[0], and the overwrite of the buffer had corrupted the
MessageRam_ structurre in the ChibiOS CAN interface code. That led to
a hardfault on receive of a CAN message
Note that this issue only happens if CAN_LOGLEVEL is set to greater
than zero, and the default is zero. So users can avoid the bug by
checking they have not changed CAN_LOGLEVEL.
Also, this is likely an issue that only happens on startup, as once
the two AP_DroneCAN threads are fully running they have the same
thread priority so can't pre-empt each other. During startup some
messages are sent from the main thread which has a different priority
to the AP_DroneCAN threads, and can thus trigger this issue
this supports logging of all bxCAN and CANFD frames, which helps with
debugging tricky CAN support issues and for the development of new CAN
driver lua scripts
keep a record of which bus we have registered a callback for and only
unregister with that bus. This prevents us unregistering a multicast
callback when disconnecting from MAVCAN
this broke when we moved to the unified AP_HAL locking system for
UARTs. The SLCAN code relied on the fact that the thread owner check
was not done for the read_locked() path. Now that we have a higher
level consistent API that check is done and SLCAN broke
Andrew Tridgell
Bob Long
bugobliterator
Peter Barker
Randy Mackay
Thomas Watson
muramura
Iampete1
rishabsingh3003
Mykhailo Kuznietsov
Tom Pittenger