USB CDC Serial loses data when sending too quickly

[ondras12345]

Describe the bug
Text printed to USB CDC Serial is received with several lines / packets missing if there is no delay between prints.

To Reproduce
Build this:

/**
 * Board: Generic STM32F1 series
 * Board part number: BluePill F103C8
 * U(S)ART support: disabled (no Serial support)
 * USB support (if available): CDC (generic 'Serial' supersede U(S)ART)
 * USB speed (if available): Low/Full Speed
 * Optimize: Smallest (-Os default)
 * Debug symbols and core logs: None
 * C Runtime Library: Newlib Nano (default)
 */

void setup()
{
    Serial.begin();

    pinMode(LED_BUILTIN, OUTPUT);
    delay(2000);
    digitalWrite(LED_BUILTIN, HIGH);
    Serial.println("boot");
    Serial.println("press t to test");
}


void loop()
{
    if (!Serial.available()) return;
    char c = Serial.read();

    if (c == 't')
    {
        Serial.println("testing");
        Serial.println("---");
        for (uint32_t i = 0; i < 5000U; i++)
        {
            Serial.printf("%u: testing... abcdefghijklmnopqrstuvwxyz\r\n", i);
        }
        Serial.println("---");
        Serial.println("done");
    }
}

Export compiled binary in Arduino IDE.

Upload using ST-Link V2:

st-flash --reset --format binary write tmp.ino.BLUEPILL_F103C8.bin 0x8000000

Steps to reproduce the behavior:

1. Upload & run, connect USB cable.
2. Attach terminal: minicom -D /dev/ttyACM0
3. Press t to run the test. Observe incomplete messages. Rerun test multiple times if no incomplete messages occur, they seem to be rare on some PCs.

Observed behavior
Data is missing in the middle:

[...]
4823: testing... abcdefghijklmnopqrstuvwxyz
4824: testing... abcdefghijklmnopqrst4994: testing... abcdefghijklmnopqrstuvwxyz
4995: testing... abcdefghijklmnopqrstuvwxyz
[...]
4999: testing... abcdefghijklmnopqrstuvwxyz
---
done

Or at the end:

[...]
4505: testing... abcdefghijklmnopqrstuvwxyz
4506: testing... abcdefghijklmnopqrstuvwxyz
4507: testing...

The likelihood of triggering the bug seems to depend on which computer I do my testing with. With two laptops (one Linux, one Windows), it occurs multiple times in each test. On another one, it is much more rare, but still should definitely occur within say 50 runs of the test.

Expected behavior
Text printed to Serial should be received by PC without error.

Desktop:

• OS: Ubuntu 22.04.3 LTS
• Arduino IDE version: 1.8.19
• STM32 core version: 2.7.1
• Tools menu settings if not the default: see source code
• Upload method: st-flash, see above

Board:

• Name: BluePill F103C8

Additional context
Here are some things I tried:

• Before I started testing with Arduino IDE, I had the same problem with platformio. I added the following build flags, without success (no combination helps):

    -D CDC_RECEIVE_QUEUE_BUFFER_PACKET_NUMBER=64
    -D CDC_TRANSMIT_QUEUE_BUFFER_PACKET_NUMBER=64
    -DSERIAL_TX_BUFFER_SIZE=128

• Waiting while(!Serial) before printing "done" does not seem to ensure it gets printed.
• Using Wireshark to capture USB traffic, I didn't see anything unusual. The packets are simply missing. No reinit. I'm not sure if Wireshark is able to show invalid packet errors. It surely does not show any.
• No usb/tty-related errors are shown in dmesg.
• I tried implementing XON/XOFF flow control. It is definitely working (tested by sending ctrl+S, ctrl+Q manually). However, the PC never sends XOFF. I believe this means it isn't a buffer overflow issue on the PC.

[fpistm]

Hi @ondras12345
USB does not guarantee to not lost packet.
Maybe related to #1399 even if you do not have fault.
You can try to force flush:

          for (uint32_t i = 0; i < 5000U; i++)
          {
              Serial.printf("%u: testing... abcdefghijklmnopqrstuvwxyz\r\n", i);
+             Serial.flush();
          }

[ondras12345]

That you for you input.

I cannot believe It didn't occur to me to try flush() before. It seems to have fixed the problem. No more lost packets.
However, it increases USB packet overhead (the message I print is shorter than the packet size, so previously a part of the next message would get sent in the same packet) and generally slows down the process (plus it is a blocking call).
I would expect print() to start blocking if the output queues are full (e.g. if the PC cannot keep up & does not send confirmation packets), but it does not seem to.

Is there a way to fix the problem while still letting CDC use all of its output queue? I believe calling flush() forces it to wait until the queue is empty after putting in just one packet.

[fpistm]

You can try to flush only %5 message or something like this.

[ondras12345]

Why does calling flush even help?
flush:

Arduino_Core_STM32/cores/arduino/USBSerial.cpp

Lines 152 to 157 in 2c45eba

	void USBSerial::flush(void)
	{
	// Wait for TransmitQueue read size becomes zero
	// TS: safe, because it not be stopped while receive 0
	while (CDC_TransmitQueue_ReadSize(&TransmitQueue) > 0) {}
	}

write:

Arduino_Core_STM32/cores/arduino/USBSerial.cpp

Lines 67 to 90 in 2c45eba

	size_t USBSerial::write(const uint8_t *buffer, size_t size)
	{
	size_t rest = size;
	while (rest > 0 && CDC_connected()) {
	// Determine buffer size available for write
	auto portion = (size_t)CDC_TransmitQueue_WriteSize(&TransmitQueue);
	// Truncate it to content size (if rest is greater)
	if (rest < portion) {
	portion = rest;
	}
	if (portion > 0) {
	// Only if some space in the buffer exists.
	// TS: Only main thread calls write and writeSize methods,
	// it's thread-safe since IRQ does not affects
	// TransmitQueue write position
	CDC_TransmitQueue_Enqueue(&TransmitQueue, buffer, portion);
	rest -= portion;
	buffer += portion;
	// After storing data, start transmitting process
	CDC_continue_transmit();
	}
	}
	return size - rest;
	}

write checks if space is available in the transmit queue and waits if there isn't, so my problem shouldn't be caused by the queue overfilling.

I think calling flush just slows down the program enough to hide a bug somewhere in the USB stack.

[fpistm]

The queue management was added by a contributor several years ago, unfortunately after several update of the USB Device middleware, it was hard to backport this implementation due to major rework of the library.
I've planned to fully rework the USB part and so will not investigate this. The time being, you can use the workaround by using flush each n packet:

          for (uint32_t i = 0; i < 5000U; i++)
          {
              Serial.printf("%u: testing... abcdefghijklmnopqrstuvwxyz\r\n", i);
+             if (i%5 == 0) {
+               Serial.flush();
+             }
+             Serial.flush();
          }