Skip to content

Commit f94d3e8

Browse files
committed
UART: Don't allocate the object so early
This object has a finalizer, so once it's no longer referenced, GC can call that finalizer and then deallocate the storage. In the case of a failure during construction (e.g., when checking `validate_obj_is_free_pin_or_none`) this will happen on an incompletely initialized structure. On samd, in particular, a newly allocated object (with construct never called) appears to be valid, so GC collecting it causes deinit() to do things, leading to a hard fault. The double creation of the UART object was necessary specifically so that the second allocation would fail. Probably there were other (single call) ways to make it fail, but this was the easiest / the one discovered in real life. Closes: #5493
1 parent b83e098 commit f94d3e8

File tree

1 file changed

+7
-6
lines changed

1 file changed

+7
-6
lines changed

shared-bindings/busio/UART.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,6 @@ STATIC void validate_timeout(mp_float_t timeout) {
8282

8383
STATIC mp_obj_t busio_uart_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) {
8484
#if CIRCUITPY_BUSIO_UART
85-
// Always initially allocate the UART object within the long-lived heap.
86-
// This is needed to avoid crashes with certain UART implementations which
87-
// cannot accomodate being moved after creation. (See
88-
// https://github.com/adafruit/circuitpython/issues/1056)
89-
busio_uart_obj_t *self = m_new_ll_obj_with_finaliser(busio_uart_obj_t);
90-
self->base.type = &busio_uart_type;
9185
enum { ARG_tx, ARG_rx, ARG_baudrate, ARG_bits, ARG_parity, ARG_stop, ARG_timeout, ARG_receiver_buffer_size,
9286
ARG_rts, ARG_cts, ARG_rs485_dir,ARG_rs485_invert};
9387
static const mp_arg_t allowed_args[] = {
@@ -140,6 +134,13 @@ STATIC mp_obj_t busio_uart_make_new(const mp_obj_type_t *type, size_t n_args, si
140134

141135
const bool rs485_invert = args[ARG_rs485_invert].u_bool;
142136

137+
// Always initially allocate the UART object within the long-lived heap.
138+
// This is needed to avoid crashes with certain UART implementations which
139+
// cannot accomodate being moved after creation. (See
140+
// https://github.com/adafruit/circuitpython/issues/1056)
141+
busio_uart_obj_t *self = m_new_ll_obj_with_finaliser(busio_uart_obj_t);
142+
self->base.type = &busio_uart_type;
143+
143144
common_hal_busio_uart_construct(self, tx, rx, rts, cts, rs485_dir, rs485_invert,
144145
args[ARG_baudrate].u_int, bits, parity, stop, timeout,
145146
args[ARG_receiver_buffer_size].u_int, NULL, false);

0 commit comments

Comments
 (0)