Skip to content

[release/6.0] Fixes #62167. WriteAsync may truncate data if called after .Advance(int) #62348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 16, 2021

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Dec 3, 2021

Backport of #62306 to release/6.0

/cc @BrennanConroy @Kuinox

Customer Impact

Customer reported issue. This can result in corrupted data (missing bytes in the middle of a Read) when using pipes with a mix of GetSpan() + Advance() and WriteAsync()

Testing

New test was written for the scenario, current test coverage gives high confidence no regressions will occur.

Risk

Low. The fix is well understood and very little code change.

@BrennanConroy BrennanConroy added the Servicing-consider Issue for next servicing release review label Dec 3, 2021
@BrennanConroy BrennanConroy added this to the 6.0.x milestone Dec 3, 2021
@Kuinox
Copy link
Contributor

Kuinox commented Dec 4, 2021

Oh nyo... The CI fails, did I broke something ?

@halter73
Copy link
Member

halter73 commented Dec 4, 2021

Oh nyo... The CI fails, did I broke something ?

No. Don't worry about it. A couple flaky Sockets tests timed out after 10 seconds, and System.Net.Sockets doesn't have a System.IO.Pipelines dependency. I'll rerun them.

@Kuinox
Copy link
Contributor

Kuinox commented Dec 4, 2021

Oh ok then ! It looked like IO write related stuff so it was close :').
Thanks!

@halter73 halter73 added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Dec 14, 2021
@safern safern force-pushed the backport/pr-62306-to-release/6.0 branch from 90dc2af to 7acde97 Compare December 15, 2021 19:17
@halter73
Copy link
Member

Thanks @safern for making the packaging changes. Do we need similar changes for the 5.0 and 3.1 backports? I'm going to be OOF starting tomorrow, so please let me know now if you need anything more from me.

@safern
Copy link
Member

safern commented Dec 15, 2021

I haven't gone through the 5.0/3.1 changes yet. I will soon, and I can help pushing the packaging changes there, so no worries about it 😄

@safern
Copy link
Member

safern commented Dec 16, 2021

Failures are unrelated. IO long running test was moved to outerloop on main: #60606

@jozkee can we port that to 6.0?

@radical System.Text.Json failed when running AOT:

emcc : error : '/datadisks/disk1/work/A6930885/p/build/emsdk/upstream/bin/wasm-opt --post-emscripten --no-exit-runtime -Oz --low-memory-unused --zero-filled-memory --strip-producers /datadisks/disk1/work/A6930885/w/B06D0928/e/wasm_build/obj/wasm/for-build/dotnet.wasm -o /datadisks/disk1/work/A6930885/w/B06D0928/e/wasm_build/obj/wasm/for-build/dotnet.wasm -g --mvp-features --enable-mutable-globals' failed (received SIGKILL (-9)) [/datadisks/disk1/work/A6930885/w/B06D0928/e/publish/ProxyProjectForAOTOnHelix.proj]
/datadisks/disk1/work/A6930885/p/build/wasm/WasmApp.Native.targets(375,5): error MSB3073: The command "emcc "@/datadisks/disk1/work/A6930885/p/build/microsoft.netcore.app.runtime.browser-wasm/runtimes/browser-wasm/native/src/emcc-default.rsp" "@/datadisks/disk1/work/A6930885/w/B06D0928/e/wasm_build/obj/wasm/for-build/emcc-link.rsp"" exited with code 1. [/datadisks/disk1/work/A6930885/w/B06D0928/e/publish/ProxyProjectForAOTOnHelix.proj]

Here is the binlog: https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-62348-merge-c7bfc92b0280452881/System.Text.Json.Tests/1/xharness-output/AOTBuild.binlog?sv=2019-07-07&se=2022-01-05T00%3A39%3A34Z&sr=c&sp=rl&sig=RKVW%2BBS58fuk1t3FmQP78NVJueTo8bvs4WQyqIIpK%2BI%3D

@safern safern merged commit 44c8586 into release/6.0 Dec 16, 2021
@safern safern deleted the backport/pr-62306-to-release/6.0 branch December 16, 2021 16:48
@jozkee
Copy link
Member

jozkee commented Dec 17, 2021

@jozkee can we port that to 6.0?

@safern sure we can, I tried "/backport to release/6.0" on #60606 but that did nothing, do you know why?

@github-actions
Copy link
Contributor Author

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1593860314

@github-actions
Copy link
Contributor Author

@jozkee an error occurred while backporting to release/6.0, please check the run log for details!

Validation Failed: {"resource":"PullRequest","code":"custom","message":"No commits between release/6.0 and backport/pr-62348-to-release/6.0"}

@jozkee
Copy link
Member

jozkee commented Dec 17, 2021

@jozkee an error occurred while backporting to release/6.0, please check the run log for details!

Validation Failed: {"resource":"PullRequest","code":"custom","message":"No commits between release/6.0 and backport/pr-62348-to-release/6.0"}

I didn't mean to backport this again, sorry.

@ghost ghost locked as resolved and limited conversation to collaborators Jan 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants