Skip to content

Commit ccd8880

Browse files
matthijskooijmancmaglie
authored andcommitted
Fix lockup when writing to HardwareSerial with interrupts disabled
When interrupts are disabled, writing to HardwareSerial could cause a lockup. When the tx buffer is full, a busy-wait loop is used to wait for the interrupt handler to free up a byte in the buffer. However, when interrupts are disabled, this will of course never happen and the Arduino will lock up. This often caused lockups when doing (big) debug printing from an interrupt handler. Additionally, calling flush() with interrupts disabled while transmission was in progress would also cause a lockup. When interrupts are disabled, the code now actively checks the UDRE (UART Data Register Empty) and calls the interrupt handler to free up room if the bit is set. This can lead to delays in interrupt handlers when the serial buffer is full, but a delay is of course always preferred to a lockup. Closes: arduino#672 References: arduino#1147
1 parent 4fb15f2 commit ccd8880

File tree

1 file changed

+21
-5
lines changed

1 file changed

+21
-5
lines changed

Diff for: hardware/arduino/avr/cores/arduino/HardwareSerial.cpp

+21-5
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,14 @@ void HardwareSerial::flush()
344344
if (!_written)
345345
return;
346346

347-
while (bit_is_set(*_ucsrb, UDRIE0) || bit_is_clear(*_ucsra, TXC0));
347+
while (bit_is_set(*_ucsrb, UDRIE0) || bit_is_clear(*_ucsra, TXC0)) {
348+
if (bit_is_clear(SREG, SREG_I) && bit_is_set(*_ucsrb, UDRIE0))
349+
// Interrupts are globally disabled, but the DR empty
350+
// interrupt should be enabled, so poll the DR empty flag to
351+
// prevent deadlock
352+
if (bit_is_set(*_ucsra, UDRE0))
353+
_tx_udr_empty_irq();
354+
}
348355
// If we get here, nothing is queued anymore (DRIE is disabled) and
349356
// the hardware finished tranmission (TXC is set).
350357
}
@@ -355,10 +362,19 @@ size_t HardwareSerial::write(uint8_t c)
355362

356363
// If the output buffer is full, there's nothing for it other than to
357364
// wait for the interrupt handler to empty it a bit
358-
// ???: return 0 here instead?
359-
while (i == _tx_buffer_tail)
360-
;
361-
365+
while (i == _tx_buffer_tail) {
366+
if (bit_is_clear(SREG, SREG_I)) {
367+
// Interrupts are disabled, so we'll have to poll the data
368+
// register empty flag ourselves. If it is set, pretend an
369+
// interrupt has happened and call the handler to free up
370+
// space for us.
371+
if(bit_is_set(*_ucsra, UDRE0))
372+
_tx_udr_empty_irq();
373+
} else {
374+
// nop, the interrupt handler will free up space for us
375+
}
376+
}
377+
362378
_tx_buffer[_tx_buffer_head] = c;
363379
_tx_buffer_head = i;
364380

0 commit comments

Comments
 (0)