Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 6ab836d

Browse files
danmoseleymaryamariyan
authored andcommitted
Try finalizer on RemoteInvokeHandle (#27087)
* Try finalizer on RemoteInvokeHandle * Adding missing Dispose call to RemoteInvoke's in different test projects * Fixing tests in System.Diagnostics.Process missing Dispose on RemoteInvokeHandle * Add missing Dispose call to System.Data.Common.XsdSchemaDeserializationIgnoresLocale() * Fixing netfx failure in System.Runtime * Adding missing Dispose call after using RemoteInvokeHandle in System.Net.ServicePoint project
1 parent ff123e8 commit 6ab836d

File tree

16 files changed

+72
-42
lines changed

16 files changed

+72
-42
lines changed

src/CoreFx.Private.TestUtilities/ref/CoreFx.Private.TestUtilities.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,9 @@ protected RemoteExecutorTestBase() { }
144144
public static System.Diagnostics.RemoteExecutorTestBase.RemoteInvokeHandle RemoteInvokeRaw(System.Delegate method, string unparsedArg, System.Diagnostics.RemoteInvokeOptions options = null) { throw null; }
145145
public sealed partial class RemoteInvokeHandle : System.IDisposable
146146
{
147-
public RemoteInvokeHandle(System.Diagnostics.Process process, System.Diagnostics.RemoteInvokeOptions options) { }
147+
public RemoteInvokeHandle(System.Diagnostics.Process process, System.Diagnostics.RemoteInvokeOptions options, string assemblyName, string className, string methodName) { }
148148
public System.Diagnostics.RemoteInvokeOptions Options { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
149-
public System.Diagnostics.Process Process { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
149+
public System.Diagnostics.Process Process { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
150150
public void Dispose() { }
151151
}
152152
}

src/CoreFx.Private.TestUtilities/src/System/Diagnostics/RemoteExecutorTestBase.Process.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@ private static RemoteInvokeHandle RemoteInvoke(MethodInfo method, string[] args,
7272
// Return the handle to the process, which may or not be started
7373
return new RemoteInvokeHandle(options.Start ?
7474
Process.Start(psi) :
75-
new Process() { StartInfo = psi }, options);
75+
new Process() { StartInfo = psi }, options,
76+
a.FullName, t.FullName, method.Name
77+
);
7678
}
7779
}
7880
}

src/CoreFx.Private.TestUtilities/src/System/Diagnostics/RemoteExecutorTestBase.cs

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,17 +186,31 @@ private static MethodInfo GetMethodInfo(Delegate d)
186186
/// <summary>A cleanup handle to the Process created for the remote invocation.</summary>
187187
public sealed class RemoteInvokeHandle : IDisposable
188188
{
189-
public RemoteInvokeHandle(Process process, RemoteInvokeOptions options)
189+
public RemoteInvokeHandle(Process process, RemoteInvokeOptions options, string assemblyName = null, string className = null, string methodName = null)
190190
{
191191
Process = process;
192192
Options = options;
193+
AssemblyName = assemblyName;
194+
ClassName = className;
195+
MethodName = methodName;
193196
}
194197

195-
public Process Process { get; private set; }
198+
public Process Process { get; set; }
196199
public RemoteInvokeOptions Options { get; private set; }
200+
public string AssemblyName { get; private set; }
201+
public string ClassName { get; private set; }
202+
public string MethodName { get; private set; }
197203

198204
public void Dispose()
199205
{
206+
Dispose(disposing: true);
207+
GC.SuppressFinalize(this);
208+
}
209+
210+
private void Dispose(bool disposing)
211+
{
212+
Assert.True(disposing, $"A test {AssemblyName}!{ClassName}.{MethodName} forgot to Dispose() the result of RemoteInvoke()");
213+
200214
if (Process != null)
201215
{
202216
// A bit unorthodox to do throwing operations in a Dispose, but by doing it here we avoid
@@ -236,6 +250,13 @@ public void Dispose()
236250
}
237251
}
238252

253+
~RemoteInvokeHandle()
254+
{
255+
// Finalizer flags tests that omitted the explicit Dispose() call; they must have it, or they aren't
256+
// waiting on the remote execution
257+
Dispose(disposing: false);
258+
}
259+
239260
private sealed class RemoteExecutionException : XunitException
240261
{
241262
internal RemoteExecutionException(string stackTrace) : base("Remote process failed with an unhandled exception.", stackTrace) { }

src/CoreFx.Private.TestUtilities/src/System/Diagnostics/RemoteExecutorTestBase.uap.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ private static RemoteInvokeHandle RemoteInvoke(MethodInfo method, string[] args,
7070
}
7171

7272
// RemoteInvokeHandle is not really needed in the UAP scenario but we use it just to have consistent interface as non UAP
73-
return new RemoteInvokeHandle(null, options);
73+
return new RemoteInvokeHandle(null, options, null, null, null);
7474
}
7575
}
7676
}

src/System.ComponentModel.TypeConverter/tests/DateTimeConverterTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public static void ConvertTo_WithContext()
6666
DateTimeConverterTests.s_converter);
6767

6868
return SuccessExitCode;
69-
});
69+
}).Dispose();
7070
}
7171
}
7272
}

src/System.ComponentModel.TypeConverter/tests/DateTimeOffsetConverterTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public static void ConvertTo_WithContext()
6868
DateTimeOffsetConverterTests.s_converter);
6969

7070
return SuccessExitCode;
71-
});
71+
}).Dispose();
7272
}
7373
}
7474
}

src/System.ComponentModel.TypeConverter/tests/TypeConverterTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ public static void ConvertTo_WithContext()
110110
Assert.NotNull(s);
111111
Assert.Equal(FormattableClass.Token, s);
112112
return SuccessExitCode;
113-
});
113+
}).Dispose();
114114
}
115115

116116
[Fact]

src/System.Data.Common/tests/System/Data/DataTableReadXmlSchemaTest.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,7 @@ public void XsdSchemaSerializationIgnoresLocale()
485485
Assert.Contains(@"AutoIncrementStep=""-2""", rawSchemaXML);
486486
Assert.DoesNotContain("()1", rawSchemaXML);
487487
Assert.DoesNotContain("()2", rawSchemaXML);
488-
});
488+
}).Dispose();
489489
}
490490

491491
[Fact]
@@ -600,7 +600,7 @@ incorrectly uses the current culture instead of the invariant culture when parsi
600600
DataColumn rowIDColumn = table.Columns["RowID"];
601601
Assert.Equal(-1, rowIDColumn.AutoIncrementSeed);
602602
Assert.Equal(-2, rowIDColumn.AutoIncrementStep);
603-
});
603+
}).Dispose();
604604
}
605605

606606
[Fact]

src/System.Diagnostics.Process/tests/ProcessTestBase.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,20 @@ protected void AddProcessForDispose(Process p)
5151

5252
protected Process CreateProcess(Func<int> method = null)
5353
{
54-
Process p = RemoteInvoke(method ?? (() => SuccessExitCode), new RemoteInvokeOptions { Start = false }).Process;
54+
RemoteInvokeHandle handle = RemoteInvoke(method ?? (() => SuccessExitCode), new RemoteInvokeOptions { Start = false });
55+
Process p = handle.Process;
56+
handle.Process = null;
57+
handle.Dispose();
5558
AddProcessForDispose(p);
5659
return p;
5760
}
5861

5962
protected Process CreateProcess(Func<string, int> method, string arg)
6063
{
61-
Process p = RemoteInvoke(method, arg, new RemoteInvokeOptions { Start = false }).Process;
64+
RemoteInvokeHandle handle = RemoteInvoke(method, arg, new RemoteInvokeOptions { Start = false });
65+
Process p = handle.Process;
66+
handle.Process = null;
67+
handle.Dispose();
6268
AddProcessForDispose(p);
6369
return p;
6470
}

src/System.IO/tests/StringWriter/StringWriterTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ public static void TestWriteMisc()
308308
sw.Write((ulong)ulong.MaxValue);
309309

310310
Assert.Equal("Truea1234.013452342.0123456-92233720368547758081234.5429496729518446744073709551615", sw.ToString());
311-
});
311+
}).Dispose();
312312
}
313313

314314
[Fact]
@@ -337,7 +337,7 @@ public static void TestWriteLineMisc()
337337
Assert.Equal(
338338
string.Format("False{0}B{0}987{0}875634{0}1.23457{0}45634563{0}18446744073709551615{0}", Environment.NewLine),
339339
sw.ToString());
340-
});
340+
}).Dispose();
341341
}
342342

343343
[Fact]

0 commit comments

Comments
 (0)