Bug 13727 – std.stdio.File not thread-safe

////////////// test.d ////////////// import std.array; import std.parallelism; import std.stdio; void main() { foreach (fn; ["test.d"] .replicate(1000) .parallel ) { // synchronized { File f = File(fn, "rb"); } } } //////////////////////////////////// On Windows 32-bit, this program will either deadlock, or print a dozen exceptions such as: std.exception.ErrnoException@std\stdio.d(640): Could not close file `test.d' (No error) Uncommenting the "synchronized" line, or commenting the ".parallel" line fixes the problem. Works fine on 64 bits (with MSVC runtime).

Note that even though the above example has all threads reading from the same file, this doesn't matter, as the problem occurs even when accessing distinct files.

I'm going to throw a shot in the dark out here, and say it's an issue with DMC runtime. Which means it's really a problem with DMC, not DMD or phobos. I'll note that the DMC runtime has a limit of 64 file descriptors, including the standard handles.

(In reply to Steven Schveighoffer from comment #2) > I'm going to throw a shot in the dark out here, and say it's an issue with > DMC runtime. Which means it's really a problem with DMC, not DMD or phobos. I agree. However, we might be able to work around thread safety issues in the DMC runtime using global locks. > I'll note that the DMC runtime has a limit of 64 file descriptors, including > the standard handles. This shouldn't be relevant except when running the test program on systems with 64 or more processors/cores.

(In reply to Vladimir Panteleev from comment #3) > (In reply to Steven Schveighoffer from comment #2) > > I'm going to throw a shot in the dark out here, and say it's an issue with > > DMC runtime. Which means it's really a problem with DMC, not DMD or phobos. > > I agree. However, we might be able to work around thread safety issues in > the DMC runtime using global locks. Oh god! More locks :) I strongly believe we should fix the issue in DMC. Can we create a C++ program that will exhibit the same issue? > > > I'll note that the DMC runtime has a limit of 64 file descriptors, including > > the standard handles. > > This shouldn't be relevant except when running the test program on systems > with 64 or more processors/cores. Oh, I didn't realize how the parallel foreach worked.

(In reply to Steven Schveighoffer from comment #4) > Oh god! More locks :) Assuming we don't need to lock for the actual I/O operations (read/write), I think the impact for locking on some O(1)-ish operations shouldn't be too bad compared to the cost of the actual I/O. > I strongly believe we should fix the issue in DMC. Can we create a C++ > program that will exhibit the same issue? Yes: ///////////////////////////// test.c //////////////////////////// #include <windows.h> #include <stdio.h> DWORD WINAPI ThreadProc(LPVOID lpParameter) { for (int i=0; i<100; i++) { int res; FILE* f = fopen("test.c", "rb"); if (!f) { printf("fopen failed\n"); return 1; } res = fclose(f); if (res) { printf("fclose failed\n"); return 1; } } return 0; } void main() { for (int i=0; i<10; i++) { DWORD dwThreadId; CreateThread(NULL, 0, &ThreadProc, NULL, 0, &dwThreadId); } } /////////////////////////////////////////////////////////////////

> I strongly believe we should fix the issue in DMC. BTW, I agree in principle, but I don't know how realistic this is, considering it's closed-source and I don't know how hard or quick it is to get a patch in.

(In reply to Vladimir Panteleev from comment #6) > > I strongly believe we should fix the issue in DMC. > > BTW, I agree in principle, but I don't know how realistic this is, > considering it's closed-source and I don't know how hard or quick it is to > get a patch in. It's on github, but private. Ask Walter for access. I fixed an issue once with pipes that was holding up std.process, so I still have access. I will warn you, though, you can't build it unless you have some obscure assembler :) At the very least, your sample code should be filed as a DMC bug, but I'm not sure where that goes. CC'ing Walter on this.

https://github.com/DigitalMars/dmc/pull/2 (access required)

Adding test case: https://github.com/dlang/dmd/pull/5747

See also: https://issues.dlang.org/show_bug.cgi?id=15768

(In reply to Vladimir Panteleev from comment #8) > https://github.com/DigitalMars/dmc/pull/2 (access required) This was pulled.

(In reply to Walter Bright from comment #11) > (In reply to Vladimir Panteleev from comment #8) > > https://github.com/DigitalMars/dmc/pull/2 (access required) > > This was pulled. issue 18483 is related and was still showing there are threading issues recently. This was pulled in May 2016, so I'm assuming dmd release has been updated with new snn.lib? In any case, there are still threading issues. Not sure whether they are all related. See also: http://bugzilla.digitalmars.com/issues/show_bug.cgi?id=327

(In reply to Steven Schveighoffer from comment #12) > This was pulled in May 2016, so I'm assuming dmd release has been > updated with new snn.lib? The test case passes, so yes. > In any case, there are still threading issues. Not > sure whether they are all related. In order to keep this open, it needs specifics.

It seems to be passing more frequently, but still seems to have issues: https://auto-tester.puremagic.com/show-run.ghtml?projectid=1&runid=3072904&isPull=true Logged output: Test runnable/test13727.d failed. The logged output: ..\generated\windows\release\32\dmd.exe -conf= -m32 -Irunnable -odtest_results\runnable -oftest_results\runnable\test13727_0.exe runnable\test13727.d -map nul.map test_results\runnable\test13727_0.exe ..\generated\windows\release\32\dmd.exe -conf= -m32 -Irunnable -inline -odtest_results\runnable -oftest_results\runnable\test13727_1.exe runnable\test13727.d -map nul.map test_results\runnable\test13727_1.exe ..\generated\windows\release\32\dmd.exe -conf= -m32 -Irunnable -release -odtest_results\runnable -oftest_results\runnable\test13727_2.exe runnable\test13727.d -map nul.map test_results\runnable\test13727_2.exe ..\generated\windows\release\32\dmd.exe -conf= -m32 -Irunnable -inline -release -odtest_results\runnable -oftest_results\runnable\test13727_3.exe runnable\test13727.d -map nul.map test_results\runnable\test13727_3.exe ..\generated\windows\release\32\dmd.exe -conf= -m32 -Irunnable -g -odtest_results\runnable -oftest_results\runnable\test13727_4.exe runnable\test13727.d -map nul.map test_results\runnable\test13727_4.exe std.exception.ErrnoException@std\stdio.d(576): Could not close file `runnable/extra-files/extra13727.txt' (No error) ---------------- 0x0040805C in @safe bool std.exception.errnoEnforce!(bool, "std\stdio.d", 576u).errnoEnforce(bool, lazy immutable(char)[]) 0x00403BCB in @safe void std.stdio.File.detach() 0x004025CD in void std.parallelism.ParallelForeach!(immutable(char)[][]).ParallelForeach.opApply(scope int delegate(ref immutable(char)[])).doIt() at C:\cygwin\home\braddr\sandbox\at-client\pull-3072904-Win_32\dmd\test\..\..\phobos\std\parallelism.d-mixin-4007(4053) 0x0040CFB1 in void std.parallelism.run!(void delegate()).run(void delegate()) 0x00404C24 in void std.parallelism.TaskPool.executeWorkLoop() 0x00431934 in __threadstartex 0x77124A77 in RtlGetAppContainerNamedObjectPath 0x77124A47 in RtlGetAppContainerNamedObjectPath

(In reply to Steven Schveighoffer from comment #14) > It seems to be passing more frequently, but still seems to have issues: I take it back, it seems this is the only time the Win32 test has run since you reopened. So seems not to be fixed.

There's currently a Phobos PR by Jack that might fix this: https://github.com/dlang/dmd/pull/5747

The new `build.d` script in dmd is running into these issues. I simplified where it's getting stuck into 3 versions with this code snippet: import std.stdio, std.algorithm, std.array, std.process, std.exception, std.parallelism; void main() { auto funcs = [() { // std.exception.ErrnoException@std\stdio.d(997): Enforcement failed (No error) version (a) ["git", "--version"].execute; // std.exception.ErrnoException@std\stdio.d(563): Could not close file `HANDLE(C0)' (No error) else version (b) ["dir"].execute; // std.exception.ErrnoException@std\stdio.d(563): Could not close file `somefile' (Bad file descriptor) else version (c) "bar".toFile("anotherfile"); else static assert(0, "Please specify -version=a, -version=b or -version=c"); }, () { "foo".toFile("somefile"); }]; foreach (func; funcs.parallel(1)) func(); }

@marler8997 created dlang/dmd pull request #9907 "Workaround issue 13727 (std.stdio.File not thread-safe)" mentioning this issue: - Workaround issue 13727 https://github.com/dlang/dmd/pull/9907

dlang/dmd pull request #9907 "Workaround issue 13727 (std.stdio.File not thread-safe)" was merged into master: - 9f7229407b56fef77dfcca50f2d3b41535fac34c by Jonathan Marler: Workaround issue 13727 https://github.com/dlang/dmd/pull/9907

@marler8997 created dlang/dmd pull request #9911 "Workaround issue 13727 only for DigitalMars CRuntime" mentioning this issue: - Workaround issue 13727 only for DigitalMars CRuntime https://github.com/dlang/dmd/pull/9911

dlang/dmd pull request #9911 "Workaround issue 13727 only for DigitalMars CRuntime" was merged into master: - 97c4d5e0b2719a29a1ce1c617d77fac3d9f86c73 by Jonathan Marler: Workaround issue 13727 only for DigitalMars CRuntime https://github.com/dlang/dmd/pull/9911

@WalterBright updated dlang/dmd pull request #5747 "fix Issue 13727 - std.stdio.File not thread-safe" fixing this issue: - fix Issue 13727 - std.stdio.File not thread-safe https://github.com/dlang/dmd/pull/5747

THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/phobos/issues/10098 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB