BusDisplay _refresh_area does not respect busio locks

[JamesWhitlock]

If the RTOS is premptive, BusDisplay has a race condition that may caused faults on shared buses. displayio_display_bus_begin_transaction may return false indicating that the lock could not be acquired. As _refresh_area does not respect the return value of displayio_display_bus_begin_transaction, _send_pixels will try to write to the bus without a lock. A guard is present in the code, but it is implemented in a way that is not thread safe.

circuitpython/shared-module/busdisplay/BusDisplay.c

Lines 290 to 297 in 117b5d1

	// Can't acquire display bus; skip the rest of the data.
	if (!displayio_display_bus_is_free(&self->bus)) {
	return false;
	}
	displayio_display_bus_begin_transaction(&self->bus);
	_send_pixels(self, (uint8_t *)buffer, subrectangle_size_bytes);
	displayio_display_bus_end_transaction(&self->bus);

This is likely responsible for issue #10536.

[JamesWhitlock]

Further incorrect handling of displayio_display_bus_begin_transaction:

circuitpython/shared-module/epaperdisplay/EPaperDisplay.c

Lines 165 to 168 in 117b5d1

	displayio_display_bus_begin_transaction(&self->bus);
	self->bus.send(self->bus.bus, DISPLAY_COMMAND, self->chip_select, cmd, 1);
	self->bus.send(self->bus.bus, DISPLAY_DATA, self->chip_select, data, data_size);
	displayio_display_bus_end_transaction(&self->bus);

There are a number of instances in displayio_display_bus_set_region_to_update, e.g.

circuitpython/shared-module/displayio/bus_core.c

Lines 131 to 141 in 117b5d1

	displayio_display_bus_begin_transaction(self);
	uint8_t data[5];
	data[0] = self->column_command;
	uint8_t data_length = 1;
	display_byte_type_t data_type = DISPLAY_DATA;
	if (!self->data_as_commands) {
	self->send(self->bus, DISPLAY_COMMAND, CHIP_SELECT_UNTOUCHED, data, 1);
	data_length = 0;
	} else {
	data_type = DISPLAY_COMMAND;
	}

[JamesWhitlock]

Example of correct usages:

circuitpython/shared-module/busdisplay/BusDisplay.c

Lines 74 to 76 in 117b5d1

	while (!displayio_display_bus_begin_transaction(&self->bus)) {
	RUN_BACKGROUND_TASKS;
	}

circuitpython/shared-module/busdisplay/BusDisplay.c

Lines 167 to 171 in 117b5d1

	ok = displayio_display_bus_begin_transaction(&self->bus);
	if (ok) {
	if (self->bus.data_as_commands) {
	uint8_t set_brightness[2] = {self->brightness_command, (uint8_t)(0xff * brightness)};
	self->bus.send(self->bus.bus, DISPLAY_COMMAND, CHIP_SELECT_TOGGLE_EVERY_BYTE, set_brightness, 2);

[tannewt]

A PR with fixes would be super helpful! You've done most of the hard work finding the incorrect uses.

[JamesWhitlock]

I'd be happy to create a PR but I've spent a couple of hours trying to get a build environment working, but haven't succeeded as of yet. I'm reluctant to submit a PR without testing it first.

[tannewt]

I'd be happy to create a PR but I've spent a couple of hours trying to get a build environment working, but haven't succeeded as of yet. I'm reluctant to submit a PR without testing it first.

Feel free to make a draft PR and let the CI build for you. You can download artifacts from the CI run to test on your boards.

When you circle back to running it locally, jump on Discord and we can help get you set up. https://adafru.it/discord