Skip to content

Commit 738cb67

Browse files
ptupitsynptupitsyn
authored
IGNITE-9638 .NET: Fix JVM thread leak - call DetachCurrentThread on CLR thread exit
* Use thread local storage destructor callback as a way to know when thread is about to exit * Tested on Windows (.NET Classic, .NET Core), Linux (.NET Core), macOS (.NET Core) * Additionally: do not call AttachCurrentThread in callbacks - we already have JniEnv pointer, use it
1 parent c5562e8 commit 738cb67

File tree

11 files changed

+480
-10
lines changed

11 files changed

+480
-10
lines changed

‎modules/core/src/test/java/org/apache/ignite/platform/PlatformThreadNamesTask.java‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.ignite.platform;
19+
20+
import org.apache.ignite.cluster.ClusterNode;
21+
import org.apache.ignite.compute.ComputeJob;
22+
import org.apache.ignite.compute.ComputeJobAdapter;
23+
import org.apache.ignite.compute.ComputeJobResult;
24+
import org.apache.ignite.compute.ComputeTaskAdapter;
25+
import org.apache.ignite.internal.util.typedef.F;
26+
import org.jetbrains.annotations.NotNull;
27+
import org.jetbrains.annotations.Nullable;
28+
29+
import java.util.Collections;
30+
import java.util.List;
31+
import java.util.Map;
32+
import java.util.Set;
33+
34+
/**
35+
* Task to get Java thread names.
36+
*/
37+
public class PlatformThreadNamesTask extends ComputeTaskAdapter<Object, String[]> {
38+
/** {@inheritDoc} */
39+
@NotNull @Override public Map<? extends ComputeJob, ClusterNode> map(List<ClusterNode> subgrid,
40+
@Nullable Object arg) {
41+
return Collections.singletonMap(new PlatformThreadNamesJob(), F.first(subgrid));
42+
}
43+
44+
/** {@inheritDoc} */
45+
@Nullable @Override public String[] reduce(List<ComputeJobResult> results) {
46+
return results.get(0).getData();
47+
}
48+
49+
/**
50+
* Job.
51+
*/
52+
private static class PlatformThreadNamesJob extends ComputeJobAdapter {
53+
/** {@inheritDoc} */
54+
@Nullable @Override public String[] execute() {
55+
Set<Thread> threadSet = Thread.getAllStackTraces().keySet();
56+
String[] threadNames = new String[threadSet.size()];
57+
58+
int i = 0;
59+
for (Thread t : threadSet)
60+
threadNames[i++] = t.getName();
61+
62+
return threadNames;
63+
}
64+
}
65+
}

‎modules/platforms/dotnet/Apache.Ignite.Core.Tests.DotNetCore/Apache.Ignite.Core.Tests.DotNetCore.csproj‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,12 @@
144144
<Compile Include="..\Apache.Ignite.Core.Tests\TestUtils.Common.cs" Link="Common\TestUtils.Common.cs" />
145145
<Compile Include="..\Apache.Ignite.Core.Tests\EnvVar.cs" Link="Common\EnvVar.cs" />
146146
<Compile Include="..\Apache.Ignite.Core.Tests\Ssl\SslConfigurationTest.cs" Link="Common\SslConfigurationTest.cs" />
147+
<Compile Include="..\Apache.Ignite.Core.Tests\Unmanaged\JniThreadDetachTest.cs">
148+
<Link>Unmanaged\JniThreadDetachTest.cs</Link>
149+
</Compile>
150+
<Compile Include="..\Apache.Ignite.Core.Tests\Unmanaged\UnmanagedThreadTest.cs">
151+
<Link>Unmanaged\UnmanagedThreadTest.cs</Link>
152+
</Compile>
147153
</ItemGroup>
148154

149155
<ItemGroup>

‎modules/platforms/dotnet/Apache.Ignite.Core.Tests/Apache.Ignite.Core.Tests.csproj‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,8 @@
337337
<Compile Include="Services\ServiceProxyTest.cs" />
338338
<Compile Include="Services\ServicesAsyncWrapper.cs" />
339339
<Compile Include="TestRunner.cs" />
340+
<Compile Include="Unmanaged\JniThreadDetachTest.cs" />
341+
<Compile Include="Unmanaged\UnmanagedThreadTest.cs" />
340342
<Compile Include="WindowsServiceTest.cs" />
341343
</ItemGroup>
342344
<ItemGroup>
@@ -534,7 +536,6 @@
534536
<None Include="packages.config" />
535537
<Compile Include="TestUtils.Windows.cs" />
536538
</ItemGroup>
537-
<ItemGroup />
538539
<Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
539540
<Target Name="AfterBuild">
540541
<Copy SourceFiles="$(SolutionDir)Apache.Ignite\bin\$(ConfigurationName)\Apache.Ignite.exe;$(SolutionDir)Apache.Ignite\bin\$(ConfigurationName)\Apache.Ignite.exe.config" DestinationFolder="$(ProjectDir)$(OutDir)" SkipUnchangedFiles="false" />

‎modules/platforms/dotnet/Apache.Ignite.Core.Tests/Unmanaged/JniThreadDetachTest.cs‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
namespace Apache.Ignite.Core.Tests.Unmanaged
19+
{
20+
using System.Linq;
21+
using NUnit.Framework;
22+
23+
/// <summary>
24+
/// Tests JVM thread detach - verify that there are no leaks caused by JNI.
25+
/// </summary>
26+
public class JniThreadDetachTest : TestBase
27+
{
28+
/// <summary>
29+
/// Tests that using Ignite APIs from CLR threads does not leak JVM threads.
30+
/// </summary>
31+
[Test]
32+
public void TestUseIgniteFromClrThreadsDoesNotLeakJvmThreads()
33+
{
34+
var cache = Ignite.GetOrCreateCache<int, int>("c");
35+
cache.Put(0, 0);
36+
37+
var threadNamesBefore = GetJavaThreadNames();
38+
39+
TestUtils.RunMultiThreaded(() => cache.Put(1, 1), 10);
40+
41+
var threadNamesAfter = GetJavaThreadNames();
42+
Assert.AreEqual(threadNamesBefore, threadNamesAfter);
43+
Assert.IsNotEmpty(threadNamesAfter);
44+
}
45+
46+
/// <summary>
47+
/// Gets Java thread names.
48+
/// </summary>
49+
private string[] GetJavaThreadNames()
50+
{
51+
return Ignite.GetCompute()
52+
.ExecuteJavaTask<string[]>("org.apache.ignite.platform.PlatformThreadNamesTask", null)
53+
.Where(x => !x.StartsWith("pub-#") && !x.StartsWith("jvm-"))
54+
.OrderBy(x => x)
55+
.ToArray();
56+
}
57+
}
58+
}

‎modules/platforms/dotnet/Apache.Ignite.Core.Tests/Unmanaged/UnmanagedThreadTest.cs‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
namespace Apache.Ignite.Core.Tests.Unmanaged
19+
{
20+
using System;
21+
using System.Runtime.InteropServices;
22+
using System.Threading;
23+
using Apache.Ignite.Core.Impl.Unmanaged;
24+
using NUnit.Framework;
25+
26+
/// <summary>
27+
/// Tests for <see cref="UnmanagedThread"/>.
28+
/// </summary>
29+
public class UnmanagedThreadTest
30+
{
31+
/// <summary>
32+
/// Tests that ThreadExit event fires when enabled
33+
/// with <see cref="UnmanagedThread.EnableCurrentThreadExitEvent"/>.
34+
/// </summary>
35+
[Test]
36+
public void TestThreadExitFiresWhenEnabled([Values(true, false)] bool enableThreadExitCallback)
37+
{
38+
var evt = new ManualResetEventSlim();
39+
var threadLocalVal = new IntPtr(42);
40+
var resultThreadLocalVal = IntPtr.Zero;
41+
42+
UnmanagedThread.ThreadExitCallback callback = val =>
43+
{
44+
evt.Set();
45+
resultThreadLocalVal = val;
46+
};
47+
48+
GC.KeepAlive(callback);
49+
var callbackId = UnmanagedThread.SetThreadExitCallback(Marshal.GetFunctionPointerForDelegate(callback));
50+
51+
try
52+
{
53+
ParameterizedThreadStart threadStart = _ =>
54+
{
55+
if (enableThreadExitCallback)
56+
UnmanagedThread.EnableCurrentThreadExitEvent(callbackId, threadLocalVal);
57+
};
58+
59+
var t = new Thread(threadStart);
60+
61+
t.Start();
62+
t.Join();
63+
64+
var threadExitCallbackCalled = evt.Wait(TimeSpan.FromSeconds(1));
65+
66+
Assert.AreEqual(enableThreadExitCallback, threadExitCallbackCalled);
67+
Assert.AreEqual(enableThreadExitCallback ? threadLocalVal : IntPtr.Zero, resultThreadLocalVal);
68+
}
69+
finally
70+
{
71+
UnmanagedThread.RemoveThreadExitCallback(callbackId);
72+
}
73+
}
74+
75+
/// <summary>
76+
/// Tests that invalid callback id causes and exception.
77+
/// </summary>
78+
[Test]
79+
public void TestInvalidCallbackIdThrowsException()
80+
{
81+
Assert.Throws<InvalidOperationException>(() =>
82+
UnmanagedThread.EnableCurrentThreadExitEvent(int.MaxValue, new IntPtr(1)));
83+
84+
Assert.Throws<InvalidOperationException>(() =>
85+
UnmanagedThread.RemoveThreadExitCallback(int.MaxValue));
86+
}
87+
}
88+
}

‎modules/platforms/dotnet/Apache.Ignite.Core/Apache.Ignite.Core.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@
9191
<Compile Include="Impl\Client\SocketEndpoint.cs" />
9292
<Compile Include="Impl\Common\TaskRunner.cs" />
9393
<Compile Include="Impl\Transactions\TransactionCollectionImpl.cs" />
94+
<Compile Include="Impl\Unmanaged\UnmanagedThread.cs" />
9495
<Compile Include="Ssl\ISslContextFactory.cs" />
9596
<Compile Include="Configuration\Package-Info.cs" />
9697
<Compile Include="Configuration\ClientConnectorConfiguration.cs" />

‎modules/platforms/dotnet/Apache.Ignite.Core/Impl/Unmanaged/Jni/Callbacks.cs‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ private void LoggerLog(IntPtr envPtr, IntPtr clazz, long igniteId, int level, In
183183
try
184184
{
185185
var cbs = _callbackRegistry.Get<UnmanagedCallbacks>(igniteId, true);
186-
var env = _jvm.AttachCurrentThread();
186+
var env = _jvm.AttachCurrentThread(envPtr);
187187

188188
var message0 = env.JStringToString(message);
189189
var category0 = env.JStringToString(category);
@@ -193,7 +193,7 @@ private void LoggerLog(IntPtr envPtr, IntPtr clazz, long igniteId, int level, In
193193
}
194194
catch (Exception e)
195195
{
196-
_jvm.AttachCurrentThread().ThrowToJava(e);
196+
_jvm.AttachCurrentThread(envPtr).ThrowToJava(e);
197197
}
198198
}
199199

@@ -211,7 +211,7 @@ private bool LoggerIsLevelEnabled(IntPtr env, IntPtr clazz, long igniteId, int l
211211
}
212212
catch (Exception e)
213213
{
214-
_jvm.AttachCurrentThread().ThrowToJava(e);
214+
_jvm.AttachCurrentThread(env).ThrowToJava(e);
215215
return false;
216216
}
217217
}
@@ -227,11 +227,13 @@ private long InLongLongLongObjectOutLong(IntPtr env, IntPtr clazz, long igniteId
227227
{
228228
var cbs = _callbackRegistry.Get<UnmanagedCallbacks>(igniteId, true);
229229

230+
_jvm.AttachCurrentThread(env);
231+
230232
return cbs.InLongLongLongObjectOutLong(op, arg1, arg2, arg3, arg);
231233
}
232234
catch (Exception e)
233235
{
234-
_jvm.AttachCurrentThread().ThrowToJava(e);
236+
_jvm.AttachCurrentThread(env).ThrowToJava(e);
235237
return 0;
236238
}
237239
}
@@ -247,11 +249,13 @@ private long InLongOutLong(IntPtr env, IntPtr clazz, long igniteId,
247249
{
248250
var cbs = _callbackRegistry.Get<UnmanagedCallbacks>(igniteId, true);
249251

252+
_jvm.AttachCurrentThread(env);
253+
250254
return cbs.InLongOutLong(op, arg);
251255
}
252256
catch (Exception e)
253257
{
254-
_jvm.AttachCurrentThread().ThrowToJava(e);
258+
_jvm.AttachCurrentThread(env).ThrowToJava(e);
255259

256260
return 0;
257261
}
@@ -276,7 +280,7 @@ private void ConsoleWrite(IntPtr envPtr, IntPtr clazz, IntPtr message, bool isEr
276280

277281
if (writer != null)
278282
{
279-
var env = _jvm.AttachCurrentThread();
283+
var env = _jvm.AttachCurrentThread(envPtr);
280284
var msg = env.JStringToString(message);
281285

282286
writer.Write(msg, isError);
@@ -285,7 +289,7 @@ private void ConsoleWrite(IntPtr envPtr, IntPtr clazz, IntPtr message, bool isEr
285289
}
286290
catch (Exception e)
287291
{
288-
_jvm.AttachCurrentThread().ThrowToJava(e);
292+
_jvm.AttachCurrentThread(envPtr).ThrowToJava(e);
289293
}
290294
}
291295
}

‎modules/platforms/dotnet/Apache.Ignite.Core/Impl/Unmanaged/Jni/Env.cs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,14 @@ public Jvm Jvm
159159
get { return _jvm; }
160160
}
161161

162+
/// <summary>
163+
/// Gets the env ptr.
164+
/// </summary>
165+
public IntPtr EnvPtr
166+
{
167+
get { return _envPtr; }
168+
}
169+
162170
/// <summary>
163171
/// Calls the static void method.
164172
/// </summary>
@@ -503,4 +511,4 @@ private static void GetDelegate<T>(IntPtr ptr, out T del)
503511
del = (T) (object) Marshal.GetDelegateForFunctionPointer(ptr, typeof(T));
504512
}
505513
}
506-
}
514+
}

‎modules/platforms/dotnet/Apache.Ignite.Core/Impl/Unmanaged/Jni/Jvm.cs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ internal sealed unsafe class Jvm
6464
/** Callbacks. */
6565
private readonly Callbacks _callbacks;
6666

67+
/** Thread exit callback id. */
68+
private readonly int _threadExitCallbackId;
69+
6770
/** Static instance */
6871
private static volatile Jvm _instance;
6972

@@ -92,7 +95,13 @@ private Jvm(IntPtr jvmPtr)
9295
var func = **funcPtr;
9396
GetDelegate(func.AttachCurrentThread, out _attachCurrentThread);
9497

98+
// JVM is a singleton, so this is one-time subscription.
99+
// This is a shortcut - we pass DetachCurrentThread pointer directly as a thread exit callback,
100+
// because signatures happen to match exactly.
101+
_threadExitCallbackId = UnmanagedThread.SetThreadExitCallback(func.DetachCurrentThread);
102+
95103
var env = AttachCurrentThread();
104+
96105
_methodId = new MethodId(env);
97106

98107
// Keep AppDomain check here to avoid JITting GetCallbacksFromDefaultDomain method on .NET Core
@@ -182,6 +191,20 @@ public Env AttachCurrentThread()
182191
}
183192

184193
_env = new Env(envPtr, this);
194+
UnmanagedThread.EnableCurrentThreadExitEvent(_threadExitCallbackId, _jvmPtr);
195+
}
196+
197+
return _env;
198+
}
199+
200+
/// <summary>
201+
/// Attaches current thread to the JVM using known envPtr and returns JNIEnv.
202+
/// </summary>
203+
public Env AttachCurrentThread(IntPtr envPtr)
204+
{
205+
if (_env == null || _env.EnvPtr != envPtr)
206+
{
207+
_env = new Env(envPtr, this);
185208
}
186209

187210
return _env;

0 commit comments

Comments
 (0)