Skip to content

Commit

Permalink
pybricks.common.ble: fix data consistency when observing
Browse files Browse the repository at this point in the history
In the `ble` module, we maintain a copy of observed advertisement data
that is updated during servicing of PBIO (contiki) events. These events
can be triggered any time the GC is invoked, e.g. during allocation of
memory for MicroPython objects.

In pb_module_ble_observe(), we were iterating over the observed data
and allocating new MicroPython objects for each item which could trigger
PBIO events to be run resulting in the observed data being modified
while we are still iterating over it. This could result in a crash, the
most likely of which is raising an exception for an invalid data type.

We can avoid this by making a copy of the data before iterating over it.

Fixes: pybricks/support#1278
  • Loading branch information
dlech committed Nov 12, 2023
1 parent 90872ab commit 462c1a6
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 7 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@

### Fixed
- Fixed Move Hub accelerometer not working since v3.3.0b5 ([support#1269]).
- Fixed Bluetooth chip locking up on Technic and City hubs when broadcasting ([[support#1095]]).
- Fixed Bluetooth chip locking up on Technic and City hubs when broadcasting ([support#1095]).
- Fixed potential crash when GC occurs while observing BLE data ([support#1278])

[support#1095]: https://github.com/pybricks/support/issues/1095
[support#1269]: https://github.com/pybricks/support/issues/1269
[support#1278]: https://github.com/pybricks/support/issues/1278

## [3.3.0b9] - 2023-10-26

Expand Down
20 changes: 14 additions & 6 deletions pybricks/common/pb_type_ble.c
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,9 @@ STATIC mp_obj_t pb_module_ble_decode(observed_data_t *data, size_t *index) {
/**
* Retrieves the last received advertising data.
*
* The data in the returned pointer is only consistent until the next PBIO event
* is processed!
*
* @param [in] channel_in Python object containing the channel number.
* @returns Pointer to the last received channel data.
* @throws ValueError If the channel is out of range.
Expand Down Expand Up @@ -417,17 +420,22 @@ STATIC observed_data_t *pb_module_ble_get_channel_data(mp_obj_t channel_in) {
*/
STATIC mp_obj_t pb_module_ble_observe(mp_obj_t self_in, mp_obj_t channel_in) {

observed_data_t *ch_data = pb_module_ble_get_channel_data(channel_in);
// BEWARE OF DRAGONS: The data returned by pb_module_ble_get_channel_data()
// is only valid until the next PBIO event is processed, which can happen
// during any MicroPython function call that allocates memory. So, we have
// to make a copy of it since we are potentially allocating multiple times
// in a loop below.
observed_data_t ch_data = *pb_module_ble_get_channel_data(channel_in);

// Have not received data yet or timed out.
if (ch_data->rssi == INT8_MIN) {
if (ch_data.rssi == INT8_MIN) {
return mp_const_none;
}

// Handle single object.
if (ch_data->size != 0 && ch_data->data[0] >> 5 == PB_BLE_BROADCAST_DATA_TYPE_SINGLE_OBJECT) {
if (ch_data.size != 0 && ch_data.data[0] >> 5 == PB_BLE_BROADCAST_DATA_TYPE_SINGLE_OBJECT) {
size_t value_index = 1;
return pb_module_ble_decode(ch_data, &value_index);
return pb_module_ble_decode(&ch_data, &value_index);
}

// Objects can be encoded in as little as one byte so we could have up to
Expand All @@ -437,11 +445,11 @@ STATIC mp_obj_t pb_module_ble_observe(mp_obj_t self_in, mp_obj_t channel_in) {
size_t index = 0;
size_t i;
for (i = 0; i < OBSERVED_DATA_MAX_SIZE; i++) {
if (index >= ch_data->size) {
if (index >= ch_data.size) {
break;
}

items[i] = pb_module_ble_decode(ch_data, &index);
items[i] = pb_module_ble_decode(&ch_data, &index);
}

return mp_obj_new_tuple(i, items);
Expand Down

0 comments on commit 462c1a6

Please sign in to comment.