`stdin`, `stdout`, and `stderr` are not `@safe`

[VPanteleev-S7]

import std.stdio;

@safe void main()
{
	stderr.writeln("Hello");
}

Fails with:

test.d(4): Error: `@safe` function `D main` cannot call `@system` function `std.stdio.makeGlobal!"core.stdc.stdio.stderr".makeGlobal`
    stderr.writeln("Hello");
    ^
/path/to/dmd.linux.nixified/dmd2/linux/bin64/../../src/phobos/std/stdio.d(#):        and using `__gshared` instead of `shared` makes it fail to infer `@safe`
    __gshared File.Impl impl;
                        ^
/path/to/dmd.linux.nixified/dmd2/linux/bin64/../../src/phobos/std/stdio.d(#):        `std.stdio.makeGlobal!"core.stdc.stdio.stderr".makeGlobal` is declared here
@property ref File makeGlobal(StdFileHandle _iob)()
                   ^

[VPanteleev-S7]

Can we do something like this?

diff --git a/std/stdio.d b/std/stdio.d
index 82a13929f..f32b5af6a 100644
--- a/std/stdio.d
+++ b/std/stdio.d
@@ -5238,8 +5238,7 @@ enum StdFileHandle: string
     stderr = "core.stdc.stdio.stderr",
 }
 
-// Undocumented but public because the std* handles are aliasing it.
-@property ref File makeGlobal(StdFileHandle _iob)()
+private ref File makeGlobal(StdFileHandle _iob)()
 {
     __gshared File.Impl impl;
     __gshared File result;
@@ -5271,6 +5270,26 @@ enum StdFileHandle: string
     return result;
 }
 
+// Undocumented but public because the std* handles are aliasing it.
+template makeSafeGlobal(StdFileHandle _iob)
+{
+    // @safe getter, @system setter
+    File makeSafeGlobal() @trusted
+    {
+        return makeGlobal!_iob();
+    }
+
+    ref File makeSafeGlobal(File value) @system
+    {
+        return makeGlobal!_iob() = value;
+    }
+
+    ref File makeSafeGlobal(ref File value) @system
+    {
+        return makeGlobal!_iob() = value;
+    }
+}
+
 /** The standard input stream.
 
     Returns:
@@ -5286,7 +5305,7 @@ enum StdFileHandle: string
         and will cause all other threads calling `read` to wait until
         the lock is released.
 */
-alias stdin = makeGlobal!(StdFileHandle.stdin);
+alias stdin = makeSafeGlobal!(StdFileHandle.stdin);
 
 ///
 @safe unittest
@@ -5324,7 +5343,7 @@ alias stdin = makeGlobal!(StdFileHandle.stdin);
         and will cause all other threads calling `write` to wait until
         the lock is released.
 */
-alias stdout = makeGlobal!(StdFileHandle.stdout);
+alias stdout = makeSafeGlobal!(StdFileHandle.stdout);
 
 ///
 @safe unittest
@@ -5387,7 +5406,7 @@ alias stdout = makeGlobal!(StdFileHandle.stdout);
         and will cause all other threads calling `write` to wait until
         the lock is released.
 */
-alias stderr = makeGlobal!(StdFileHandle.stderr);
+alias stderr = makeSafeGlobal!(StdFileHandle.stderr);
 
 ///
 @safe unittest

Reading without a lock is probably not memory safe, but would require the "cooperation" of @system code to actually result in bad memory...

[rikkimax]

I've thought about this a bit, for standard IO handles, these won't ever go away.

However, other handles that people may pass in might.

Those aliases would have to go away, in favor of dedicated functions, then makeSafeGlobal can be private.

[VPanteleev-S7]

I've thought about this a bit, for standard IO handles, these won't ever go away.

Generally speaking, not being able to write to stderr from @safe code is not acceptable. Something else must give, whether that's adding a new @safe public symbol such as a @safe getStderr() or writelnErr, or a breaking change (though this change could be in the form of a new major Phobos version).

One simple solution could be to make stderr etc. thread-local. Yes, a breaking change, but I doubt there are many D applications which sorely rely on overriding the std.stdio File objects (specifically at that level, and not descriptors nor C FILE objects nor Windows HANDLEs), and doing so for all threads. Projects which we predict to be affected (maybe e.g. unit-threaded) could prepare by using a non-shared static constructor.

[VPanteleev-S7]

By the way, I checked what std.logger does, as it provides a @safe interface for writing to standard error, and... there are 31 occurrences of @trusted in just std.logger.core.

[rikkimax]

That isn't my concern, it's all about the lifetime of the handle itself.

We can't detect this scenario where it dies, and we still have a wrapper for it.

EDIT: due to reuse of handles by the kernel this is a real risk of bad things happening.

[VPanteleev-S7]

I don't understand how that's related to the issue at hand. First, that's not a memory safety issue. Second, we're talking about the standard three file descriptors, which we can generally assume are always valid.

[rikkimax]

Yes, those three are hard-coded id's, they are perfectly safe to wrap.

The problem is that the function can take in others and allow you to wrap it in @safe code.

[VPanteleev-S7]

the function

Sorry, what function? makeGlobal is an undocumented internal.

[rikkimax]

makeSafeGlobal

[VPanteleev-S7]

From the patch above? Also undocumented internal (with makeGlobal now properly private).

[pbackus]

@rikkimax I think this safety issue can be solved by adding a template constraint or static assert to makeSafeGlobal which ensures that StdFileHandle _iob is one of StdFileHandle.stdin, StdFileHandle.stdout, or StdFileHandle.stderr.

[rikkimax]

@rikkimax I think this safety issue can be solved by adding a template constraint or static assert to makeSafeGlobal which ensures that StdFileHandle _iob is one of StdFileHandle.stdin, StdFileHandle.stdout, or StdFileHandle.stderr.

Works for me.