Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Fix FileStream.FlushAsync() to behave like Flush() #24902

Merged
merged 1 commit into from
May 31, 2019

Conversation

stephentoub
Copy link
Member

Flush() behaves like Flush(false) and writes out any buffered data but doesn't P/Invoke to FlushFileBuffers/FSync to flush the OS buffers.

But whereas FlushAsync() is supposed to just be an async equivalent of Flush() (that's what every consumer of a Stream expects), it's actually behaving like Flush(true). This makes FlushAsync() inconsistent and much more expensive. (This is separate from FlushAsync not actually being async, which is an impactful problem to be solved separately.)

This changes FlushAsync to behave like Flush()/Flush(false) rather than Flush(true). If someone wants the FlushFileBuffers/FSync behavior, they can call Flush(true).

@JeremyKuhne, @jkotas, are you ok with this?

Flush() behaves like Flush(false) and writes out any buffered data but doesn't P/Invoke to FlushFileBuffers/FSync to flush the OS buffers.

But whereas FlushAsync() is supposed to just be an async equivalent of Flush(), it's actually behaving like Flush(true).  This makes FlushAsync() inconsistent and much more expensive.  (This is separate from FlushAsync not actually being async, which is an impactful problem to be solved separately.)

This changes FlushAsync to behave like Flush()/Flush(false) rather than Flush(true).  If someone wants the FlushFileBuffers/FSync behavior, they can call Flush(true).
Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@stephentoub
Copy link
Member Author

Great. Thanks.

rpetrusha pushed a commit to dotnet/dotnet-api-docs that referenced this pull request May 31, 2019
@stephentoub stephentoub merged commit 436debb into dotnet:master May 31, 2019
@stephentoub stephentoub deleted the flushasyncfs branch May 31, 2019 23:42
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corert that referenced this pull request May 31, 2019
)

Flush() behaves like Flush(false) and writes out any buffered data but doesn't P/Invoke to FlushFileBuffers/FSync to flush the OS buffers.

But whereas FlushAsync() is supposed to just be an async equivalent of Flush(), it's actually behaving like Flush(true).  This makes FlushAsync() inconsistent and much more expensive.  (This is separate from FlushAsync not actually being async, which is an impactful problem to be solved separately.)

This changes FlushAsync to behave like Flush()/Flush(false) rather than Flush(true).  If someone wants the FlushFileBuffers/FSync behavior, they can call Flush(true).

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/mono that referenced this pull request May 31, 2019
)

Flush() behaves like Flush(false) and writes out any buffered data but doesn't P/Invoke to FlushFileBuffers/FSync to flush the OS buffers.

But whereas FlushAsync() is supposed to just be an async equivalent of Flush(), it's actually behaving like Flush(true).  This makes FlushAsync() inconsistent and much more expensive.  (This is separate from FlushAsync not actually being async, which is an impactful problem to be solved separately.)

This changes FlushAsync to behave like Flush()/Flush(false) rather than Flush(true).  If someone wants the FlushFileBuffers/FSync behavior, they can call Flush(true).

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
marek-safar pushed a commit to mono/mono that referenced this pull request Jun 1, 2019
)

Flush() behaves like Flush(false) and writes out any buffered data but doesn't P/Invoke to FlushFileBuffers/FSync to flush the OS buffers.

But whereas FlushAsync() is supposed to just be an async equivalent of Flush(), it's actually behaving like Flush(true).  This makes FlushAsync() inconsistent and much more expensive.  (This is separate from FlushAsync not actually being async, which is an impactful problem to be solved separately.)

This changes FlushAsync to behave like Flush()/Flush(false) rather than Flush(true).  If someone wants the FlushFileBuffers/FSync behavior, they can call Flush(true).

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
jkotas pushed a commit to dotnet/corert that referenced this pull request Jun 2, 2019
)

Flush() behaves like Flush(false) and writes out any buffered data but doesn't P/Invoke to FlushFileBuffers/FSync to flush the OS buffers.

But whereas FlushAsync() is supposed to just be an async equivalent of Flush(), it's actually behaving like Flush(true).  This makes FlushAsync() inconsistent and much more expensive.  (This is separate from FlushAsync not actually being async, which is an impactful problem to be solved separately.)

This changes FlushAsync to behave like Flush()/Flush(false) rather than Flush(true).  If someone wants the FlushFileBuffers/FSync behavior, they can call Flush(true).

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corefx that referenced this pull request Jun 7, 2019
)

Flush() behaves like Flush(false) and writes out any buffered data but doesn't P/Invoke to FlushFileBuffers/FSync to flush the OS buffers.

But whereas FlushAsync() is supposed to just be an async equivalent of Flush(), it's actually behaving like Flush(true).  This makes FlushAsync() inconsistent and much more expensive.  (This is separate from FlushAsync not actually being async, which is an impactful problem to be solved separately.)

This changes FlushAsync to behave like Flush()/Flush(false) rather than Flush(true).  If someone wants the FlushFileBuffers/FSync behavior, they can call Flush(true).

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
stephentoub added a commit to dotnet/corefx that referenced this pull request Jun 7, 2019
)

Flush() behaves like Flush(false) and writes out any buffered data but doesn't P/Invoke to FlushFileBuffers/FSync to flush the OS buffers.

But whereas FlushAsync() is supposed to just be an async equivalent of Flush(), it's actually behaving like Flush(true).  This makes FlushAsync() inconsistent and much more expensive.  (This is separate from FlushAsync not actually being async, which is an impactful problem to be solved separately.)

This changes FlushAsync to behave like Flush()/Flush(false) rather than Flush(true).  If someone wants the FlushFileBuffers/FSync behavior, they can call Flush(true).

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
)

Flush() behaves like Flush(false) and writes out any buffered data but doesn't P/Invoke to FlushFileBuffers/FSync to flush the OS buffers.

But whereas FlushAsync() is supposed to just be an async equivalent of Flush(), it's actually behaving like Flush(true).  This makes FlushAsync() inconsistent and much more expensive.  (This is separate from FlushAsync not actually being async, which is an impactful problem to be solved separately.)

This changes FlushAsync to behave like Flush()/Flush(false) rather than Flush(true).  If someone wants the FlushFileBuffers/FSync behavior, they can call Flush(true).

Commit migrated from dotnet/coreclr@436debb
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants