Skip to content

Conversation

thelvyn
Copy link

@thelvyn thelvyn commented Aug 8, 2025

asyncio generic fails when the buffer is larger than the file contents.

Description

  1. testasyncio was updated so it fails when the error occurs using asyncio generic (works fine with liburing, not tested on windows)
  2. the fix is trivial: if the read output is <= to the read request size, then we return SDL_ASYNCIO_COMPLETE. This seems to match the expected behavior.

@icculus
Copy link
Collaborator

icculus commented Aug 9, 2025

A short read would be an error, if it isn't EOF, though. Let me look at this more closely before we merge.

@thelvyn
Copy link
Author

thelvyn commented Aug 9, 2025

The problem I encountered was that the IO remained in READY state after the read instead of EOF (even if EOF was reached).

My use case is to reuse buffers and don't query the file size before reading, that's usual with primitives like read. My logic is based on the fact that transferred_bytes < buffer_size or transferred_bytes==0 is considered as EOF so I'm done.

I agree that it is not strictly correct in case of spurious failures, I should wait for a EOF flag anyway 👍
An incomplete read is not necessarily a failure or a EOF, just that a second read should be performed to get more data or EOF (or a EAGAIN I suppose).

I can investigate this further if you want, or leave it to you.

@icculus icculus self-assigned this Aug 13, 2025
@icculus
Copy link
Collaborator

icculus commented Aug 26, 2025

Okay, I looked into this, and there's definitely a bug; SDL_IOStream won't change its status from SDL_IO_STATUS_READY until it can't make progress...so if there was an error or an EOF but you got 5 bytes out of it first, the read will return 5 and the status will remain READY, until you try to read again, in which case you'll get zero and EOF/ERROR/NOT_READY/whatever.

This seems incorrect (fread() with a return > 0 can still have feof() return nonzero, SDL should probably match this), but also risky to change; it looks like all the backends do it this way at the moment.

However, testing for this case in the caller sort of sucks--you have to try to read another byte and see what happens--so we probably should correct this in SDL_IOStream and leave the generic asyncio as it is.

(It also checks if SDL_GetError() returns something other than "" to decide if there was an error, and that's probably worth cleaning up as well.)

I'm going to close this PR; let's move the discussion back to #13720.

@icculus icculus closed this Aug 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants