floooh / sokol

minimal cross-platform standalone C headers
https://floooh.github.io/sokol-html5
zlib License
6.63k stars 472 forks source link

fecth response callback called with 'cancelled' flag but not the 'finished' one #881

Closed guillaumeblanc closed 10 months ago

guillaumeblanc commented 10 months ago

Hi,

I modified sokol fetch unit tests to reproduce a functional behavior that doesn't match with documentation. When sfetch_cancel is called between sfetch_send and sfetch_dowork, then the response callback is called with cancelled flag but not finished. This behavior differs from documentation and makes fetch callback implementation less straightforward.

I couldn't find a fix yet. Maybe there's a better way to implement this test with the existing ones.

Side note: Thanks for the incredibly well thought, easy to use and yet very efficient sokol APIs.

Regards, Guillaume

floooh commented 10 months ago

Good catch! I actually need to debug a bit what happens in sfetch_dowork with such 'early-cancelled' request.

Normally this code should take care of the problem:

https://github.com/floooh/sokol/blob/d4ac122f36d7659a18b312fd4fa2317fb9e06a63/sokol_fetch.h#L2500-L2503

...but the response callback may already be called before that here (when no buffer is provided with the request):

https://github.com/floooh/sokol/blob/d4ac122f36d7659a18b312fd4fa2317fb9e06a63/sokol_fetch.h#L2475-L2477

I think such "early cancelled" requests should be dealt with already in the first loop (https://github.com/floooh/sokol/blob/d4ac122f36d7659a18b312fd4fa2317fb9e06a63/sokol_fetch.h#L2467-L2479), so that such requests are not even dispatched into the user_incoming ring buffer (instead call the response callback with finished+cancelled and free the item right away).

I'll try to look into it in the next few days.

floooh commented 10 months ago

I'm closing this PR in favour of https://github.com/floooh/sokol/pull/898 (besides fixing the issue I also replicated your test, and also added another test which checks behaviour when cancelling after the dispatch from outside the response callback).