You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nuttx.apache.org by Nathan Hartman <ha...@gmail.com> on 2022/07/13 18:25:51 UTC

Serial UART O_NONBLOCK stale data after close()

In uart_close() of drivers/serial/serial.c, we have this:

  /* There are no more references to the port */

  dev->open_count = 0;

  /* Stop accepting input */

  uart_disablerxint(dev);

  /* Prevent blocking if the device is opened with O_NONBLOCK */

  if ((filep->f_oflags & O_NONBLOCK) == 0)
    {
      /* Now we wait for the transmit buffer(s) to clear */

      uart_tcdrain(dev, false, 4 * TICK_PER_SEC);
    }

This means that when the UART is closed() by calling close(), any
pending TX bytes still in the queue will be transmitted and we will
block here (up to 4 seconds) to wait for it. That's OK.

However, suppose the UART was opened with O_NONBLOCK. Then we do not
run uart_tcdrain(), because that would block. But the very next code
does this:

  /* Free the IRQ and disable the UART */

  flags = enter_critical_section();  /* Disable interrupts */
  uart_detach(dev);                  /* Detach interrupts */
  if (!dev->isconsole)               /* Check for the serial console UART */
    {
      uart_shutdown(dev);            /* Disable the UART */
    }

  leave_critical_section(flags);

So now the UART interrupts are gone, and nothing will move any more
characters from the TX queue to the UART. Those characters will just
sit there in the TX queue indefinitely. When the next program opens
the UART and wants to send data, those old stale characters will
finally be transmitted before the new data.

Does it make more sense, if O_NONBLOCK, to erase the contents of the
RX and TX queues so the next program does not begin with old stale
data?

So, something like this:

  if ((filep->f_oflags & O_NONBLOCK) == 0)
    {
      /* Now we wait for the transmit buffer(s) to clear */

      uart_tcdrain(dev, false, 4 * TICK_PER_SEC);
    }
  else
    {
      memset(dev->xmit, 0, sizeof(dev->xmit));
      memset(dev->recv, 0, sizeof(dev->recv));
    }

Cheers,
Nathan