Skip to content

[release/9.0-staging] [Android] Prevent race condition by synchronizing buffer access #124629

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.

Sign up for GitHub

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

Open
kotlarmilos wants to merge 3 commits into
base: release/9.0-staging
Choose a base branch
from
+76 −37
Open

[release/9.0-staging] [Android] Prevent race condition by synchronizing buffer access #124629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d594f7
[Android] Prevent race condition by synchronizing buffer access (#117…
kotlarmilos Jul 31, 2025
120fb25
Exclude WeakGCHandle changes
kotlarmilos Feb 20, 2026
ac89abe
Nit
kotlarmilos Feb 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 75 additions & 36 deletions ...libraries/System.Net.Security/src/System/Net/Security/Pal.Android/SafeDeleteSslContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Security.Authentication;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.Threading;

using PAL_KeyAlgorithm = Interop.AndroidCrypto.PAL_KeyAlgorithm;
using PAL_SSLStreamStatus = Interop.AndroidCrypto.PAL_SSLStreamStatus;
Expand All @@ -29,13 +30,17 @@ internal sealed class SafeDeleteSslContext : SafeDeleteContext

private readonly SafeSslHandle _sslContext;

private readonly Lock _lock = new Lock();

private ArrayBuffer _inputBuffer = new ArrayBuffer(InitialBufferSize);
private ArrayBuffer _outputBuffer = new ArrayBuffer(InitialBufferSize);

public SslStream.JavaProxy SslStreamProxy { get; }

public SafeSslHandle SslContext => _sslContext;

private volatile bool _disposed;

public SafeDeleteSslContext(SslAuthenticationOptions authOptions)
: base(IntPtr.Zero)
{
Expand All @@ -59,13 +64,21 @@ public SafeDeleteSslContext(SslAuthenticationOptions authOptions)

protected override void Dispose(bool disposing)
{
if (disposing)
lock (_lock)
{
if (_sslContext is SafeSslHandle sslContext)
if (!_disposed)
{
_inputBuffer.Dispose();
_outputBuffer.Dispose();
sslContext.Dispose();
_disposed = true;

// First dispose the SSL context to trigger native cleanup
_sslContext.Dispose();

if (disposing)
{
// Then dispose the buffers
_inputBuffer.Dispose();
_outputBuffer.Dispose();
}
}
}

Expand All @@ -78,11 +91,19 @@ private static unsafe void WriteToConnection(IntPtr connection, byte* data, int
SafeDeleteSslContext? context = (SafeDeleteSslContext?)GCHandle.FromIntPtr(connection).Target;
Debug.Assert(context != null);

var inputBuffer = new ReadOnlySpan<byte>(data, dataLength);
lock (context._lock)
{
if (context._disposed)
{
return;
}

var inputBuffer = new ReadOnlySpan<byte>(data, dataLength);

context._outputBuffer.EnsureAvailableSpace(dataLength);
inputBuffer.CopyTo(context._outputBuffer.AvailableSpan);
context._outputBuffer.Commit(dataLength);
context._outputBuffer.EnsureAvailableSpace(dataLength);
inputBuffer.CopyTo(context._outputBuffer.AvailableSpan);
context._outputBuffer.Commit(dataLength);
}
}

[UnmanagedCallersOnly]
Expand All @@ -91,45 +112,60 @@ private static unsafe PAL_SSLStreamStatus ReadFromConnection(IntPtr connection,
SafeDeleteSslContext? context = (SafeDeleteSslContext?)GCHandle.FromIntPtr(connection).Target;
Debug.Assert(context != null);

int toRead = *dataLength;
if (toRead == 0)
return PAL_SSLStreamStatus.OK;

if (context._inputBuffer.ActiveLength == 0)
lock (context._lock)
{
*dataLength = 0;
return PAL_SSLStreamStatus.NeedData;
}
if (context._disposed)
{
*dataLength = 0;
return PAL_SSLStreamStatus.Error;
}

toRead = Math.Min(toRead, context._inputBuffer.ActiveLength);
int toRead = *dataLength;
if (toRead == 0)
return PAL_SSLStreamStatus.OK;

context._inputBuffer.ActiveSpan.Slice(0, toRead).CopyTo(new Span<byte>(data, toRead));
context._inputBuffer.Discard(toRead);
if (context._inputBuffer.ActiveLength == 0)
{
*dataLength = 0;
return PAL_SSLStreamStatus.NeedData;
}

toRead = Math.Min(toRead, context._inputBuffer.ActiveLength);

context._inputBuffer.ActiveSpan.Slice(0, toRead).CopyTo(new Span<byte>(data, toRead));
context._inputBuffer.Discard(toRead);

*dataLength = toRead;
return PAL_SSLStreamStatus.OK;
*dataLength = toRead;
return PAL_SSLStreamStatus.OK;
}
}

internal void Write(ReadOnlySpan<byte> buf)
{
_inputBuffer.EnsureAvailableSpace(buf.Length);
buf.CopyTo(_inputBuffer.AvailableSpan);
_inputBuffer.Commit(buf.Length);
lock (_lock)
{
_inputBuffer.EnsureAvailableSpace(buf.Length);
buf.CopyTo(_inputBuffer.AvailableSpan);
_inputBuffer.Commit(buf.Length);
}
}

internal int BytesReadyForConnection => _outputBuffer.ActiveLength;

internal void ReadPendingWrites(ref ProtocolToken token)
{
if (_outputBuffer.ActiveLength == 0)
lock (_lock)
{
token.Size = 0;
token.Payload = null;
return;
}
if (_outputBuffer.ActiveLength == 0)
{
token.Size = 0;
token.Payload = null;
return;
}

token.SetPayload(_outputBuffer.ActiveSpan);
_outputBuffer.Discard(_outputBuffer.ActiveLength);
token.SetPayload(_outputBuffer.ActiveSpan);
_outputBuffer.Discard(_outputBuffer.ActiveLength);
}
}

internal int ReadPendingWrites(byte[] buf, int offset, int count)
Expand All @@ -139,12 +175,15 @@ internal int ReadPendingWrites(byte[] buf, int offset, int count)
Debug.Assert(count >= 0);
Debug.Assert(count <= buf.Length - offset);

int limit = Math.Min(count, _outputBuffer.ActiveLength);
lock (_lock)
{
int limit = Math.Min(count, _outputBuffer.ActiveLength);

_outputBuffer.ActiveSpan.Slice(0, limit).CopyTo(new Span<byte>(buf, offset, limit));
_outputBuffer.Discard(limit);
_outputBuffer.ActiveSpan.Slice(0, limit).CopyTo(new Span<byte>(buf, offset, limit));
_outputBuffer.Discard(limit);

return limit;
return limit;
}
}

private static SafeSslHandle CreateSslContext(SslStream.JavaProxy sslStreamProxy, SslAuthenticationOptions authOptions)
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/tests.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@
<ProjectExclusions Remove="$(MSBuildThisFileDirectory)System.Collections\tests\System.Collections.Tests.csproj" />
<ProjectExclusions Remove="$(MSBuildThisFileDirectory)System.IO.IsolatedStorage\tests\System.IO.IsolatedStorage.Tests.csproj" />
</ItemGroup>

<ItemGroup Condition="'$(TestNativeAot)' == 'true' or '$(TargetsMobile)' == 'true'">
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Resources.Extensions\tests\BinaryFormatTests\System.Resources.Extensions.BinaryFormat.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Resources.Extensions\tests\CompatTests\System.Resources.Extensions.Compat.Tests.csproj" />
Expand Down
Loading