fix Issue 13727 - std.stdio.File not thread-safe

[WalterBright]

The fix is actually here DigitalMars/dmc#2 (access required) and needs a new DMC runtime library snn.lib

https://issues.dlang.org/show_bug.cgi?id=13727

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	13727	major	std.stdio.File not thread-safe

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#5747"

[WalterBright]

This will fail on Win32 until the autotester starts using the new snn.lib

[MetaLang]

What's with the weird indentation?

[yebblies]

Can't this be done trivially without depending on phobos?

[braddr]

Since there's been a demonstrated failure of the test without the fixed snn.lib now, I've updated the two windows auto-tester hosts. I agree that depending on phobos in a dmd test is less than ideal, but even less ideal than that is no test. Since there's disagreement about the implementation of the test, I won't trigger the auto-merge, but we definitely want a test for this bug fix.

[yebblies]

Since druntime contains portable threading, it should be straightforward to implement without the phobos dependency.

[wilzbach]

Can't this be done trivially without depending on phobos?

What's so bad about depending on Phobos? I heard a lot of stories from people complaining about not being able to use Phobos and that you want to depend on Phobos for dmd in the future...

[dnadlinger]

Why not just put this test into Phobos?

[yebblies]

What's so bad about depending on Phobos? I heard a lot of stories from people complaining about not being able to use Phobos and that you want to depend on Phobos for dmd in the future...

It makes debugging and reducing regressions much more difficult, and the test cases much more fragile. This has nothing to do with dmd not using phobos internally.

[andralex]

@WalterBright yah let's move this to Phobos. Thx!

[schveiguy]

Can we install this test in the right place? Apologies if it already has, but I don't know how to look for it. I realized the D bug referenced above is still not closed, even though the fix was pulled in dmc, that should probably be closed, but the test should be there first.

[CyberShadow]

Or simply translate the C example to D instead of using the D/Phobos one.

[WalterBright]

Reopened because the test still isn't anywhere. Closing it without creating another PR with the test in it just causes it to be forgotten, not fixed.

[schveiguy]

It seems like the problem hasn't really been fixed, there are sporadic errors on Win32 clients.

[ibuclaw]

Have you tried raising a PR in phobos?

[ibuclaw]

To use a true story as an anecdote, druntime found a malloc bug in glibc, but the regression test for the glibc bug fix was not added to the D compiler testsuite. And why would it?

[schveiguy]

Yes, we technically should depend on DMC adding the regression test. But is DMC being CI tested? Is it being actively developed? And is it fun to write C tests when you can write D ones? ;)

glibc is probably a much different story.

I'd much rather the test be here (or at least somewhere in a dlang repository), but more importantly, it seems the fix hasn't worked.

[wilzbach]

I'd much rather the test be here (or at least somewhere in a dlang repository), but more importantly, it seems the fix hasn't worked.

FYI the auto-tester hosts use a three year old host compiler:

DMD64 D Compiler v2.068.2 Copyright (c) 1999-2015 

So the buggy dmc and snn.lib are very likely still on the auto-tester.

[schveiguy]

@wilzbach but that doesn't matter, what only matters is the snn.lib that is being linked against Phobos/druntime. The host compiler is just used to build dmd itself.

Is there a way to check which version phobos is being linked against?

[wilzbach]

@wilzbach but that doesn't matter, what only matters is the snn.lib that is being linked against Phobos/druntime.

Isn't snn.lib shipped with DMD on Windows, so I assume that if an old DMD is still there than the similarly old snn.lib will be there too.

Is there a way to check which version phobos is being linked against?

DMD is built without Phobos. Most other commands (except for the dmd build tools in test use the freshly built version of Phobos)
I'm not sure what you are referring to?

[schveiguy]

I assume that if an old DMD is still there than the similarly old snn.lib will be there too.

Not necessarily. We want to ensure we can build DMD with specific versions of DMD for bootstrapping, especially since you would a C++-based version of DMD to build DMD from scratch, so we aren't updating the compiler. That doesn't mean we can't update the snn.lib that is used to link Phobos.

[schveiguy]

DMD is built without Phobos.

Right, but this test DOES use phobos. And is using the built compiler to test it. But it could be done without Phobos, see @CyberShadow's example in the bug report.

Whether it's built with or without phobos, it's LINKED against phobos, which is linked against snn.lib. Based on @braddr's comment, I think the snn.lib has already been updated. And this is still failing.

[wilzbach]

Rebased to master to see whether this is still failing.
edit: looks like Walter has disabled pushing to his PRs.

[WalterBright]

rebased

[thewilsonator]

Removing auto merge - this still fails tests

Seriously no.

[ibuclaw]

No Phobos tests in the compiler testsuite.