Skip to content

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

Merged
safern merged 2 commits into
release/6.0from
backport/pr-62306-to-release/6.0
Dec 16, 2021
Merged

[release/6.0] Fixes #62167. WriteAsync may truncate data if called after .Advance(int)#62348
safern merged 2 commits into
release/6.0from
backport/pr-62306-to-release/6.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Dec 3, 2021

Copy link
Copy Markdown
Contributor

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.

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

Kuinox commented Dec 4, 2021

Copy link
Copy Markdown
Contributor

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

@halter73

Copy link
Copy Markdown
Member

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
Copy Markdown
Contributor

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

@halter73halter73 added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Dec 14, 2021
@safern
safernforce-pushed the backport/pr-62306-to-release/6.0 branch from 90dc2af to 7acde97CompareDecember 15, 2021 19:17
@halter73

Copy link
Copy Markdown
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
Copy Markdown
Member

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
Copy Markdown
Member

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.0Dec 16, 2021
@safern
safern deleted the backport/pr-62306-to-release/6.0 branch December 16, 2021 16:48
@jozkee

jozkee commented Dec 17, 2021

Copy link
Copy Markdown
Member

@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
Copy Markdown
ContributorAuthor

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

@github-actions

Copy link
Copy Markdown
ContributorAuthor

@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
Copy Markdown
Member

@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.

@ghostghost locked as resolved and limited conversation to collaborators Jan 17, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.IO.PipelinesServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@Kuinox@halter73@safern@jozkee@BrennanConroy