Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

response.Body.FlushAsync does nothing until 1024 bytes written to body #1092

Closed
The-MAZZTer opened this issue Sep 7, 2016 · 9 comments
Closed

Comments

@The-MAZZTer
Copy link

The-MAZZTer commented Sep 7, 2016

I searched through the source code to try and explain this behavior. Closest thing I found is: #1061 ,which is a change for a future release I don't have.

I don't think that is what I'm seeing though, as that describes a 64k buffer and it applies to the whole stream, and it keeps buffering after the first write. I've confirmed what I'm seeing happens until 1k, never happens afterwards, and adding response headers does not affect it.

Here a minimal ActionResult's ExecuteResultAsync, which should be all you need to reproduce:

        public async override Task ExecuteResultAsync(ActionContext context) {
            HttpResponse response = context.HttpContext.Response;
            response.Headers.Add("Content-Encoding", "none");
            response.ContentType = "text/plain";
            while (true) {
                if (context.HttpContext.RequestAborted.IsCancellationRequested) {
                    return;
                }

                try {
                    await response.WriteAsync("1234567890");
                    await response.Body.FlushAsync();
                } catch (Exception) {
                    try {
                        response.Body.Dispose();
                    } catch (Exception) { }
                }

                await Task.Delay(100);
            }
        }

I've only recently discovered you can use async functions for routes... this is what I hacked together not knowing that. Should still work. Works fine past 1kb of content.

Only workaround I've figured out is to spit out 1kb of garbage data before any real data which isn't ideal.

I am using the .NET Core 1.0 release, running on Windows 7 x64. My project is using Kestrel.

[Edit: Going to try .NET Core source stepping assuming that's a thing you can do and see what the code is doing that way.]

[Edit: I traced the code all the way down to Microsoft.AspNetCore.Server.Kestrel.Internal.Networking.Libuv.write. Both calls seem identical before and after 1024 bytes have been written; I have no clue what's different.]

@benaadams
Copy link
Contributor

benaadams commented Sep 8, 2016

I think I have identified the issue and raised a separate one that encapsulates it #1093

Btw you don't need to Dispose the Body stream as it is a null operation; but you also probably shouldn't continue to infinitely loop after you have tried to dispose the Body stream; or swallow all exceptions and continue to infinitely loop... (As your function will likely never exit)

@davidfowl
Copy link
Member

Btw you don't need to Dispose the Body stream as it is a null operation;

You shouldn't dispose streams you didn't create.

@benaadams
Copy link
Contributor

benaadams commented Sep 8, 2016

Btw you don't need to Dispose the Body stream as it is a null operation;

You shouldn't dispose streams you didn't create.

Ah, yes might do bad thing if was SSL, GZip, buffer stream, etc

@The-MAZZTer
Copy link
Author

Alright, thanks. I think I got that from StackOverflow, and I probably used .NET 4.5 code and modified it to work on .NET Core. I assumed at some point RequestAborted would get set. I'll replace the Dispose with a return statement.

Glad to hear you were able to reproduce and have an idea to fix. Is there a workaround I can do in the meantime? Something I can tweak through Reflection? Or something quick and dirty in the source I can modify and recompile?

@khellang
Copy link
Contributor

khellang commented Sep 8, 2016

I assumed at some point RequestAborted would get set.

Just some info on RequestAborted; aspnet/Mvc#5239 (comment)

@benaadams
Copy link
Contributor

benaadams commented Sep 8, 2016

Glad to hear you were able to reproduce and have an idea to fix. Is there a workaround I can do in the meantime?

Flush will flush the stream; the issue is on the server you regain the ability to write before the flush is complete.

Eventually it will block and pause the writes so I'm not sure it will cause a huge issue, in that you shouldn't end up with a large backlog of items to be written; and the flush will be actioned.

@The-MAZZTer
Copy link
Author

The-MAZZTer commented Sep 8, 2016

I think we are talking about different issues.

Whether I call Flush or not doesn't change the behavior of the sample code I posted; the problem I am seeing can be reproduced without it.

Scenarios I tested:

  • I write out less than 1024 bytes to the response body and return from ExecuteResultAsync immediately. The output is then immediately sent to the browser and I can see it.
  • I write out more than 1024 bytes right away to the response and don't return from ExecuteResultAsync, but endlessly loop. The output shows up in the browser right away. Any future WriteAsync calls show up right away.
  • I write out less than 1024 bytes right away to the response and don't return from ExecuteResultAsync, but endlessly loop. The output does not show up in the browser until the output accumulates to 1024 bytes. Chrome's Network Developer Tool claims 0 bytes received on the connection until that point. It doesn't matter if I change the delay between WriteAsync calls, or change the number of bytes I am writing at once, it's based on the number of bytes I've written, not some function of time.
  • If I write 10 bytes every 100ms it takes about 10 seconds before any output shows up in the browser (and then everything written comes through at once), then every 100ms after that 10 more bytes show up.

Other things I tested:

  • Adding additional headers to the response does not change the required 1024 bytes in the body. This suggests to me whatever is happening is happening on the HTTP protocol handling level (or higher) rather than the socket level.
  • Including or not including the FlushAsync call has no effect on the output.
  • Using Body.WriteAsync instead of response.WriteAsync is no different.
  • Substituting an async controller for a ActionResult gives the same result:
        [Route("test"), HttpGet]
        public async Task Test() {
            HttpResponse response = this.HttpContext.Response;
            response.Headers.Add("Content-Encoding", "none");
            response.ContentType = "text/plain";
            while (true) {
                if (this.HttpContext.RequestAborted.IsCancellationRequested) {
                    return;
                }

                try {
                    await response.WriteAsync("1234567890");
                    await response.Body.FlushAsync();
                } catch {
                    return;
                }

                await Task.Delay(100);
            }
        }

As a side note I am navigating to my API endpoints using Google Chrome and seeing this behavior (latest Stable and Dev both work).

@benaadams
Copy link
Contributor

Its more likely that Chrome is trying is waiting for the end of the response; what does dev tools show you on the time to first byte? Are you going via a reverse proxy (e.g. IIS/ngnix)

@The-MAZZTer
Copy link
Author

The-MAZZTer commented Sep 8, 2016

I just tested using PowerShell's Invoke-WebRequest and it immediately starts counting bytes returned, so it does seem to be a Chrome/Firefox issue. >_<

Chrome tools indeed shows the response starts immediately, though the tools themselves don't show this data until it starts displaying the file, which threw me.

https://stackoverflow.com/questions/35073009/disabling-chrome-buffering-when-streaming-text-data

Changing my Content-Type to something other than text/plain works (I used text/event-stream). Sorry about that, I can't use Wireshark on my work PC, my usual HTTP test client doesn't work with an endless push response, and I didn't think about trying PowerShell until just now.

At least I helped you guys find an unrelated bug.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants