Skip to content

[Android] Prevent race condition by synchronizing buffer access #117950

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

Merged
kotlarmilos merged 19 commits into from
Jul 31, 2025
Merged

[Android] Prevent race condition by synchronizing buffer access #117950

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
cc97c25
Dispose SafeSslHandle and use thread-safe operations on PAL layer
kotlarmilos Jul 22, 2025
9c1b904
Refactor SSL stream handling to remove atomic operations and use Safe…
kotlarmilos Jul 22, 2025
1d36a5e
Remove newline
kotlarmilos Jul 22, 2025
e5524a6
Improve thread safety during disposal
kotlarmilos Jul 22, 2025
0c18e08
Revert changes
kotlarmilos Jul 22, 2025
b561265
Merge branch 'main' into bugfix/android-coreclr-ssl-context
kotlarmilos Jul 22, 2025
5ff5dfb
Fix disposal logic in SafeDeleteSslContext to prevent double disposal…
kotlarmilos Jul 23, 2025
063f5d4
Update SafeDeleteSslContext to manage GC handles using a static Concu…
kotlarmilos Jul 23, 2025
c813751
Update src/libraries/System.Net.Security/src/System/Net/Security/Pal.…
kotlarmilos Jul 23, 2025
4525c6d
Eliminate GC handle ConcurrentDictionary
kotlarmilos Jul 23, 2025
66dfb1f
Add managed context cleanup
kotlarmilos Jul 23, 2025
e228f91
Improve exception handling in WriteToConnection and ReadFromConnectio…
kotlarmilos Jul 24, 2025
e95bc03
Replace object lock with class
kotlarmilos Jul 24, 2025
e38433f
Always release native _sslContext
kotlarmilos Jul 24, 2025
d35aa7e
Improve error handling and refactor WriteToConnection and ReadFromCon…
kotlarmilos Jul 25, 2025
9fd0e8e
Fix formatting
kotlarmilos Jul 25, 2025
38e45d7
Update src/libraries/System.Net.Security/src/System/Net/Security/Pal.…
kotlarmilos Jul 28, 2025
f8a6b05
Replace handle.Free() with handle.Dispose()
kotlarmilos Jul 28, 2025
020ac2d
Refactor SSL stream cleanup
kotlarmilos Jul 30, 2025
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
135 changes: 96 additions & 39 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 @@ -29,13 +29,19 @@ internal sealed class SafeDeleteSslContext : SafeDeleteContext

private readonly SafeSslHandle _sslContext;

private GCHandle _gcHandle;

private readonly object _lock = new object();

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 @@ -45,6 +51,7 @@ public SafeDeleteSslContext(SslAuthenticationOptions authOptions)
try
{
_sslContext = CreateSslContext(SslStreamProxy, authOptions);
_gcHandle = GCHandle.Alloc(this);
InitializeSslContext(_sslContext, authOptions);
}
catch (Exception ex)
Expand All @@ -59,13 +66,20 @@ public SafeDeleteSslContext(SslAuthenticationOptions authOptions)

protected override void Dispose(bool disposing)
{
if (disposing)
if (disposing && !_disposed)
{
if (_sslContext is SafeSslHandle sslContext)
_disposed = true;

lock (_lock)
{
_inputBuffer.Dispose();
_outputBuffer.Dispose();
sslContext.Dispose();
_sslContext.Dispose();
}

if (_gcHandle.IsAllocated)
{
_gcHandle.Free();
}
}

Expand All @@ -75,61 +89,100 @@ protected override void Dispose(bool disposing)
[UnmanagedCallersOnly]
private static unsafe void WriteToConnection(IntPtr connection, byte* data, int dataLength)
{
SafeDeleteSslContext? context = (SafeDeleteSslContext?)GCHandle.FromIntPtr(connection).Target;
Debug.Assert(context != null);
GCHandle h = GCHandle.FromIntPtr(connection);
SafeDeleteSslContext? context = h.IsAllocated ? h.Target as SafeDeleteSslContext : null;
if (context is null || context._disposed)
{
Debug.Write("WriteToConnection: context is null");
return;
}

var inputBuffer = new ReadOnlySpan<byte>(data, dataLength);
try
{
lock (context._lock)
{
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);
}
}
catch (Exception ex)
{
Debug.Write("Exception Caught. - " + ex);
}
}

[UnmanagedCallersOnly]
private static unsafe PAL_SSLStreamStatus ReadFromConnection(IntPtr connection, byte* data, int* dataLength)
{
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)
GCHandle h = GCHandle.FromIntPtr(connection);
SafeDeleteSslContext? context = h.IsAllocated ? h.Target as SafeDeleteSslContext : null;
if (context is null || context._disposed)
{
Debug.Write("ReadFromConnection: context is null");
*dataLength = 0;
return PAL_SSLStreamStatus.NeedData;
return PAL_SSLStreamStatus.Error;
}

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

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

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

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

*dataLength = toRead;
return PAL_SSLStreamStatus.OK;
}
}
catch (Exception ex)
{
Debug.Write("Exception Caught. - " + ex);
*dataLength = 0;
return PAL_SSLStreamStatus.Error;
}
}

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 +192,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 Expand Up @@ -227,9 +283,10 @@ private unsafe void InitializeSslContext(

// Make sure the class instance is associated to the session and is provided
// in the Read/Write callback connection parameter
IntPtr managedContextHandle = GCHandle.ToIntPtr(GCHandle.Alloc(this, GCHandleType.Weak));
string? peerHost = !isServer && !string.IsNullOrEmpty(authOptions.TargetHost) ? authOptions.TargetHost : null;
Interop.AndroidCrypto.SSLStreamInitialize(handle, isServer, managedContextHandle, &ReadFromConnection, &WriteToConnection, InitialBufferSize, peerHost);

IntPtr contextHandle = GCHandle.ToIntPtr(_gcHandle);
Interop.AndroidCrypto.SSLStreamInitialize(handle, isServer, contextHandle, &ReadFromConnection, &WriteToConnection, InitialBufferSize, peerHost);

if (authOptions.EnabledSslProtocols != SslProtocols.None)
{
Expand Down
3 changes: 1 addition & 2 deletions src/libraries/tests.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,6 @@
</ItemGroup>

<ItemGroup Condition="'$(TargetOS)' == 'android' and '$(RuntimeFlavor)' == 'CoreCLR' and '$(RunDisabledAndroidTests)' != 'true'">
<!-- https://github.com/dotnet/runtime/issues/117045 -->
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Net.Http\tests\FunctionalTests\System.Net.Http.Functional.Tests.csproj" />
<!-- https://github.com/dotnet/runtime/issues/114951 -->
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Diagnostics.DiagnosticSource\tests\TestWithConfigSwitches\System.Diagnostics.DiagnosticSource.Switches.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Runtime\tests\System.Resources.ResourceManager.Tests\System.Resources.ResourceManager.Tests.csproj" />
Expand Down Expand Up @@ -604,6 +602,7 @@
<SmokeTestProject Include="$(MSBuildThisFileDirectory)System.Diagnostics.Tracing\tests\System.Diagnostics.Tracing.Tests.csproj" />
<SmokeTestProject Include="$(MSBuildThisFileDirectory)System.Security.Cryptography\tests\System.Security.Cryptography.Tests.csproj" />
<SmokeTestProject Include="$(MSBuildThisFileDirectory)System.Net.WebSockets.Client\tests\System.Net.WebSockets.Client.Tests.csproj" />
<SmokeTestProject Include="$(MSBuildThisFileDirectory)System.Net.Http\tests\FunctionalTests\System.Net.Http.Functional.Tests.csproj" />
<SmokeTestProject Include="$(RepoRoot)\src\tests\FunctionalTests\Android\Device_Emulator\JIT\*.Test.csproj"/>
</ItemGroup>

Expand Down
Loading