aspnet/AspNetWebStack

PushStreamContent has unclear documentation

lichutin-st opened this issue · 3 comments

Hello!

As for me, documentation for a constructor of PushStreamContent which receives an asynchronous action looks a bit unclear.

It's said The stream is automatically closed when the return task is completed but actually it isn't. Here code waits for TaskSource.Task completition that is only caused by manual calling stream.Dispose(): link

So this content will never be completed

new PushStreamContent(async (stream, content, transport) => {
    await JsonSerializer.SerializeAsync(stream, data);
});

And we have to close the stream manually to make it work

new PushStreamContent(async (stream, content, transport) => {
    await JsonSerializer.SerializeAsync(stream, data);
    stream.Dispose();
});

You can compare the constructor with another one (receiving sync action). Docs there look more correct

Thanks for contacting us.
We've reviewed the code and the task is indeed being closed. Note, that the lambda function that you're calling stream.Dispose() in is meant to simply use the stream and the disposal will happen in a different place, not within the scope of this code. It will happen when the work with the stream is complete.

Hello!
@mkArtakMSFT I'm not sure I fully got your response. If I don't dispose the stream inside the lambda, the request will never be completed.

            var content = new PushStreamContent(async (stream, content, transport) => {
                await JsonSerializer.SerializeAsync(stream, new { Field1 = "Value1", Field2 = "Value2" });
            });

            var client = new HttpClient();

            Console.WriteLine("Before http request");

            var response = await client.PostAsync("https://google.com", content);

            Console.WriteLine("After http request");

"After http request" is never printed. Disposal never happen in that approach.

@lichutin-st you're right, PushStreamContent does wait for the stream to be closed / disposed. I can look at updating the documentation here.