runtime: Path.GetTempFileName() sometimes fails on WASM due to insufficient randomness

Build, seen on multiple PRs with no related changes. Which suggests that this is on main. Failures like:

[19:23:32] info: Starting:    System.CodeDom.Tests.dll
[19:23:39] fail: [FAIL] System.CodeDom.Compiler.Tests.CodeCompilerTests.FromSource_ValidSource_ReturnsExpected(source: "")
[19:23:39] info: System.IO.IOException : File exists
[19:23:39] info:    at Interop.ThrowExceptionForIoErrno(ErrorInfo errorInfo, String path, Boolean isDirectory)
[19:23:39] info:    at Interop.ThrowIOExceptionForLastError()
[19:23:39] info:    at System.IO.Directory.CreateTempSubdirectoryCore(String prefix)
[19:23:39] info:    at System.IO.Directory.CreateTempSubdirectory(String prefix)
[19:23:39] info:    at System.CodeDom.Compiler.TempFileCollection.GetTempDirectory()
[19:23:39] info:    at System.CodeDom.Compiler.TempFileCollection.EnsureTempNameCreated()
[19:23:39] info:    at System.CodeDom.Compiler.TempFileCollection.get_BasePath()
[19:23:39] info:    at System.CodeDom.Compiler.TempFileCollection.AddExtension(String fileExtension, Boolean keepFile)
[19:23:39] info:    at System.CodeDom.Compiler.TempFileCollection.AddExtension(String fileExtension)
[19:23:39] info:    at System.CodeDom.Compiler.CodeCompiler.FromSourceBatch(CompilerParameters options, String[] sources)
[19:23:39] info:    at System.CodeDom.Compiler.CodeCompiler.FromSource(CompilerParameters options, String source)
[19:23:39] info:    at System.CodeDom.Compiler.Tests.CodeCompilerTests.Compiler.FromSourceEntryPoint(CompilerParameters options, String source)
[19:23:39] info:    at System.CodeDom.Compiler.Tests.CodeCompilerTests.FromSource_ValidSource_ReturnsExpected(String source)
[19:23:39] info:    at System.Reflection.MethodInvoker.InterpretedInvoke(Object obj, Span`1 args, BindingFlags invokeAttr)

… and …

[19:27:13] info: Starting:    System.Diagnostics.TextWriterTraceListener.Tests.dll
[19:27:14] fail: [FAIL] System.Diagnostics.TextWriterTraceListenerTests.CtorsDelimiterTests.TestConstructorWithFileNameAndName(testName: "")
[19:27:14] info: System.IO.IOException : File exists
[19:27:14] info:    at Interop.ThrowExceptionForIoErrno(ErrorInfo errorInfo, String path, Boolean isDirectory)
[19:27:14] info:    at Interop.CheckIo(Int64 result, String path, Boolean isDirectory)
[19:27:14] info:    at Interop.CheckIo(IntPtr result, String path, Boolean isDirectory)
[19:27:14] info:    at System.IO.Path.GetTempFileName()
[19:27:14] info:    at System.Diagnostics.TextWriterTraceListenerTests.CtorsDelimiterTests.TestConstructorWithFileNameAndName(String testName)
[19:27:14] info:    at System.Reflection.MethodInvoker.InterpretedInvoke(Object obj, Span`1 args, BindingFlags invokeAttr)
[19:27:14] fail: [FAIL] System.Diagnostics.TextWriterTraceListenerTests.CtorsDelimiterTests.TestConstructorWithFileNameAndName(testName: "><&")
[19:27:14] info: System.IO.IOException : File exists
[19:27:14] info:    at Interop.ThrowExceptionForIoErrno(ErrorInfo errorInfo, String path, Boolean isDirectory)
[19:27:14] info:    at Interop.CheckIo(Int64 result, String path, Boolean isDirectory)
[19:27:14] info:    at Interop.CheckIo(IntPtr result, String path, Boolean isDirectory)
[19:27:14] info:    at System.IO.Path.GetTempFileName()
[19:27:14] info:    at System.Diagnostics.TextWriterTraceListenerTests.CtorsDelimiterTests.TestConstructorWithFileNameAndName(String testName)
[19:27:14] info:    at System.Reflection.MethodInvoker.InterpretedInvoke(Object obj, Span`1 args, BindingFlags invokeAttr)

Just for people reading this in the future, the list of failed tests:

System.CodeDom.Compiler.Tests.CodeCompilerTests.FromDom_ValidCodeCompileUnit_ReturnsExpected(compilationUnit: CodeCompileUnit { AssemblyCustomAttributes = [], EndDirectives = [], Namespaces = [], ReferencedAssemblies = ["assembly1", "assembly2"], StartDirectives = [], ... })
System.CodeDom.Compiler.Tests.CodeCompilerTests.GetResponseFileCmdArgs_ValidCmdArgs_ReturnsExpected(cmdArgs: "")
System.CodeDom.Compiler.Tests.CodeCompilerTests.CompileAssemblyFromSourceBatch_ValidSources_ReturnsExpected(sources: [null])
System.CodeDom.Compiler.Tests.CodeCompilerTests.FromSource_ValidSource_ReturnsExpected(source: "")
System.CodeDom.Compiler.Tests.CodeCompilerTests.FromSourceBatch_ValidSources_ReturnsExpected(sources: [""])
System.CodeDom.Compiler.Tests.CodeCompilerTests.FromSource_ValidSource_ThrowsPlatformNotSupportedException(source: "")
System.CodeDom.Compiler.Tests.CodeCompilerTests.FromDom_ValidCodeCompileUnit_ThrowsPlatformNotSupportedException(compilationUnit: CodeCompileUnit { AssemblyCustomAttributes = [], EndDirectives = [], Namespaces = [], ReferencedAssemblies = ["referenced", "assembly1"], StartDirectives = [], ... })
System.CodeDom.Compiler.Tests.CodeCompilerTests.FromSourceBatch_ValidSources_ThrowsPlatformNotSupportedException(sources: [""])

System.Diagnostics.TextWriterTraceListenerTests.CtorsDelimiterTests.TestConstructorWithFileNameAndName(testName: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"...)
System.Diagnostics.TextWriterTraceListenerTests.CtorsDelimiterTests.TestConstructorWithFileNameAndName(testName: "><&")
System.Diagnostics.TextWriterTraceListenerTests.XmlWriterTraceListenerTests.SingleArgumentConstructorTest

 ystem.ServiceModel.Syndication.Tests.BasicScenarioTests.SyndicationFeed_Write_RSS_Atom

The full set can be seen with that build url. I think https://github.com/dotnet/runtime/pull/73408 broke these tests.

@eerhardt

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 47 (47 by maintainers)

Commits related to this issue

[wasm] Disable some tests on NodeJS/Windows Issue: https://github.com/dotnet/runtime/issues/73721 — committed to radical/runtime by radical 2 years ago
Fix some randomly failing Perf.File tests on wasm/aot On wasm/aot, `System.IO.FileSystem/Perf.File`'s `CopyTo`, `CopyToOverride`, and `ReadAllBytes` benchmarks fail randomly with: ``` ---> System.IO.... — committed to radical/performance by radical 2 years ago
Fix some randomly failing Perf.File tests on wasm/aot (#2601) On wasm/aot, `System.IO.FileSystem/Perf.File`'s `CopyTo`, `CopyToOverride`, and `ReadAllBytes` benchmarks fail randomly with: ``` ---> ... — committed to dotnet/performance by radical 2 years ago

Most upvoted comments

The random function that is used is different

This gave me some insight as to why #73408 affected Path.GetTempFileName() on WASM and why it started failing. In the emscripten version, the random function uses:

	r = ts.tv_nsec*65537 ^ (uintptr_t)&ts / 16 + (uintptr_t)template;

Note that last part: + (uintptr_t)template, they are adding the pointer value into the randomness.

Looking at my change in #73408:

-            byte[] name = Encoding.UTF8.GetBytes(template);
+            string tempPath = Path.GetTempPath();
+            int tempPathByteCount = Encoding.UTF8.GetByteCount(tempPath);
+            int totalByteCount = tempPathByteCount + fileTemplate.Length + 1;

+            Span<byte> path = totalByteCount <= 256 ? stackalloc byte[256].Slice(0, totalByteCount) : new byte[totalByteCount];

Previously, we were always allocating the byte[] on the heap. Thus the pointer value was more than likely different every time we called GetTempFileName(). But, in order to reduce allocations, I changed the function to stackalloc the byte[], which means if GetTempFileName() is called twice in a row from the same method, the pointer value is going to be the same, because it will occupy the same space on the stack.

eerhardt on Aug 17, 2022

We could also use a WASM specific C# method that always called Encoding.UTF8.GetBytes on the string. This would allocate memory on the heap, which would change the template pointer passed into the __randname function every time GetTempFileName was called.

eerhardt on Sep 7, 2022

@pavelsavara do we need to fix this in 7? The console template uses node, but it is still in wasm-experimental.

radical on Aug 12, 2022

busy wait ? no thank you 😃

It’s better to throw an exception?

@tmds @omajid - do you happen to know how many retries a “normal” mkdtemp implementation tries before giving up? 100 retries seems to be awfully small.

eerhardt on Aug 11, 2022

Also note that CreateTempSubdirectoryCore ought to be passing the path to ThrowExceptionForIoErrno so that it appears in the error message.

danmoseley on Aug 11, 2022