From ccd8880a37261b53ae11c666de0a29d85c28ae36 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 18 Apr 2013 21:34:00 +0200 Subject: [PATCH] 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: #672 References: #1147 --- .../avr/cores/arduino/HardwareSerial.cpp | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/hardware/arduino/avr/cores/arduino/HardwareSerial.cpp b/hardware/arduino/avr/cores/arduino/HardwareSerial.cpp index 1bd3bff12..ffdb82c94 100644 --- a/hardware/arduino/avr/cores/arduino/HardwareSerial.cpp +++ b/hardware/arduino/avr/cores/arduino/HardwareSerial.cpp @@ -344,7 +344,14 @@ void HardwareSerial::flush() if (!_written) return; - while (bit_is_set(*_ucsrb, UDRIE0) || bit_is_clear(*_ucsra, TXC0)); + while (bit_is_set(*_ucsrb, UDRIE0) || bit_is_clear(*_ucsra, TXC0)) { + if (bit_is_clear(SREG, SREG_I) && bit_is_set(*_ucsrb, UDRIE0)) + // Interrupts are globally disabled, but the DR empty + // interrupt should be enabled, so poll the DR empty flag to + // prevent deadlock + if (bit_is_set(*_ucsra, UDRE0)) + _tx_udr_empty_irq(); + } // If we get here, nothing is queued anymore (DRIE is disabled) and // the hardware finished tranmission (TXC is set). } @@ -355,10 +362,19 @@ size_t HardwareSerial::write(uint8_t c) // If the output buffer is full, there's nothing for it other than to // wait for the interrupt handler to empty it a bit - // ???: return 0 here instead? - while (i == _tx_buffer_tail) - ; - + while (i == _tx_buffer_tail) { + if (bit_is_clear(SREG, SREG_I)) { + // Interrupts are disabled, so we'll have to poll the data + // register empty flag ourselves. If it is set, pretend an + // interrupt has happened and call the handler to free up + // space for us. + if(bit_is_set(*_ucsra, UDRE0)) + _tx_udr_empty_irq(); + } else { + // nop, the interrupt handler will free up space for us + } + } + _tx_buffer[_tx_buffer_head] = c; _tx_buffer_head = i;