Skip to content

Commit

Permalink
Fix Issue 15768 - std.stdio.trustedStdout accesses __gshared data wit…
Browse files Browse the repository at this point in the history
…hout synchronization
  • Loading branch information
JackStouffer committed Apr 3, 2018
1 parent 861e65a commit 9410ed0
Showing 1 changed file with 36 additions and 32 deletions.
68 changes: 36 additions & 32 deletions std/stdio.d
Expand Up @@ -369,14 +369,15 @@ Hello, Jimmy!
*/
struct File
{
import core.atomic : atomicOp, atomicStore, atomicLoad;
import std.range.primitives : ElementEncodingType;
import std.traits : isScalarType, isArray;
enum Orientation { unknown, narrow, wide }

private struct Impl
{
FILE * handle = null; // Is null iff this Impl is closed by another File
uint refs = uint.max / 2;
shared uint refs = uint.max / 2;
bool isPopened; // true iff the stream has been created by popen()
Orientation orientation;
}
Expand All @@ -397,7 +398,7 @@ struct File
{
assert(_p);
_p.handle = handle;
_p.refs = refs;
atomicStore(_p.refs, refs);
_p.isPopened = isPopened;
_p.orientation = Orientation.unknown;
_name = name;
Expand Down Expand Up @@ -473,8 +474,8 @@ Throws: `ErrnoException` if the file could not be opened.
this(this) @safe nothrow
{
if (!_p) return;
assert(_p.refs);
++_p.refs;
assert(atomicLoad(_p.refs));
atomicOp!"+="(_p.refs, 1);
}

/**
Expand Down Expand Up @@ -513,14 +514,11 @@ Throws: `ErrnoException` in case of error.
{
_p = cast(Impl*) enforce(malloc(Impl.sizeof), "Out of memory");
}
else if (_p.refs == 1)
{
closeHandles();
}
else
{
_p.refs--;
_p = cast(Impl*) enforce(malloc(Impl.sizeof), "Out of memory");
if (atomicOp!"-="(_p.refs, 1) == 0)
closeHandles();
else
_p = cast(Impl*) enforce(malloc(Impl.sizeof), "Out of memory");
}

FILE* handle;
Expand Down Expand Up @@ -840,16 +838,17 @@ Detaches from the underlying file. If the sole owner, calls `close`.
Throws: `ErrnoException` on failure if closing the file.
*/
void detach() @safe
void detach() @trusted
{
import core.stdc.stdlib : free;

if (!_p) return;
if (_p.refs == 1)
close();
else
scope(exit) _p = null;

if (atomicOp!"-="(_p.refs, 1) == 0)
{
assert(_p.refs);
--_p.refs;
_p = null;
scope(exit) free(_p);
closeHandles();
}
}

Expand Down Expand Up @@ -887,8 +886,7 @@ Throws: `ErrnoException` on error.
if (!_p) return; // succeed vacuously
scope(exit)
{
assert(_p.refs);
if (!--_p.refs)
if (atomicOp!"-="(_p.refs, 1) == 0)
free(_p);
_p = null; // start a new life
}
Expand Down Expand Up @@ -3427,6 +3425,24 @@ void main()
assert(e && e.msg == "Attempting to write to closed File");
}

version(StdStressTest)
{
// issue 15768
@system unittest
{
import std.parallelism : parallel;
import std.range : iota;

auto deleteme = testFilename();
stderr = File(deleteme, "w");

foreach (t; 1_000_000.iota.parallel)
{
stderr.write("aaa");
}
}
}

/// Used to specify the lock type for `File.lock` and `File.tryLock`.
enum LockType
{
Expand Down Expand Up @@ -4691,10 +4707,6 @@ enum StdFileHandle: string
}

/** The standard input stream.
Bugs:
Due to $(LINK2 https://issues.dlang.org/show_bug.cgi?id=15768, bug 15768),
it is thread un-safe to reassign `stdin` to a different `File` instance
than the default.
Returns: stdin as a $(LREF File).
*/
Expand Down Expand Up @@ -4722,10 +4734,6 @@ alias stdin = makeGlobal!(StdFileHandle.stdin);

/**
The standard output stream.
Bugs:
Due to $(LINK2 https://issues.dlang.org/show_bug.cgi?id=15768, bug 15768),
it is thread un-safe to reassign `stdout` to a different `File` instance
than the default.
Returns: stdout as a $(LREF File).
*/
Expand Down Expand Up @@ -4778,10 +4786,6 @@ alias stdout = makeGlobal!(StdFileHandle.stdout);

/**
The standard error stream.
Bugs:
Due to $(LINK2 https://issues.dlang.org/show_bug.cgi?id=15768, bug 15768),
it is thread un-safe to reassign `stderr` to a different `File` instance
than the default.
Returns: stderr as a $(LREF File).
*/
Expand Down

0 comments on commit 9410ed0

Please sign in to comment.