[release/3.1] Port fix to revert EncoderNLS and DecoderNLS Convert changes

[GrabYourPitchforks]

Port dotnet/runtime#752 to release/3.1
Fixes dotnet/runtime#594

Description

In .NET Core 3.0 / 3.1 we changed how the completed out parameter is set on the EncoderNLS.Convert and DecoderNLS.Convert routines. We had changed the logic to match precisely what MSDN's documentation stated and to fix a possible data corruption bug; however, this broke a number of customers' applications by sending them into infinite loops.

This PR reverts that change.

Customer Impact

If customers were relying on the previous behavior of the completed out parameter, their applications could end up in an infinite loop. What makes matters worse is that the sample code on the MSDN page for Encoder.Convert and Decoder.Convert is also susceptible to this pattern, so we suspect a large number of customers may have based their own application code on the sample.

Regression?

Yes, from .NET Framework 4.x and .NET Core 2.x. One half of this change was introduced in .NET Core 3.0 (see 43a5159); the other half was introduced in .NET Core 3.1 (see c07ec4c).

Testing

The reversion is already committed in the 5.0 branches (see dotnet/runtime@d49b541), so we have been getting test coverage on those nightlies. No notable regressions have been reported.

Risk

Somewhere between minimal and moderate. It's possible that some applications may have taken a dependency on the new behavior. See for instance dotnet/aspnetcore#17747, where ASP.NET appears to have unintentionally taken a dependency on the new behavior and had to proactively make a change so that they behave correctly both before and after this PR goes through.

/cc @tarekgh @anurse @danmosemsft

[danmoseley]

Should we also port the test changes I see in dotnet/runtime#752 (unfortunately a separate PR we could only commit later when coreclr flowed)

[jamshedd]

Approved for Feb.

[GrabYourPitchforks]

Tests didn't seem to be running in CoreFX (at least on my box). I'm going to update the .rsp file here to account for test failures.

Note: the .rsp file update is not a functional product change. It only affects how CI runs unit tests.

[danmoseley]

Yeah, I think the tests PR into corefx could be merged later (week?) after CoreCLR flows into CoreFX. So it's not time sensitive.

[danmoseley]

Processor affinity test failure, clearly unrelated:

Assert failure(PID 6491 [0x0000195b], Thread: 6491 [0x195b]): currentProcessCpuCount == g_processAffinitySet.Count()
    File: /__w/1/s/src/vm/gcenv.os.cpp Line: 114
    Image: /home/helixbot/work/B6C60A33/p/dotnet

    System.Diagnostics.Tests.ProcessTests.TestProcessorAffinity [FAIL]
      System.InvalidOperationException : Cannot process request because the process (6491) has exited.
      Stack Trace:
        /_/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs(343,0): at System.Diagnostics.Process.ThrowIfExited(Boolean refresh)
        /_/src/System.Diagnostics.Process/src/System/Diagnostics/Process.cs(937,0): at System.Diagnostics.Process.EnsureState(State state)
        /_/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs(193,0): at System.Diagnostics.Process.set_ProcessorAffinityCore(IntPtr value)
        /_/src/System.Diagnostics.Process/tests/ProcessTests.cs(865,0): at System.Diagnostics.Tests.ProcessTests.TestProcessorAffinity()

[danmoseley]

@VSadov is it worth an issue in dotnet/runtime for the processor affinity assertion failure above? Note this is in release/3.1. Maybe something was fixed in master since. The test is simply setting processor affinity to 0x1 and then setting it back. It's on Ubuntu1804.

[danmoseley]

@GrabYourPitchforks I think you can go ahead and merge this if you're satisfied the Corefx tests pass with it (or at least pass with locally making the same rollback in the tests)