ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
34.33k stars 2.51k forks source link

std.debug.warn hangs when no stderr is attached on windows #4196

Open Rageoholic opened 4 years ago

Rageoholic commented 4 years ago

If you run an app from the command line it will function as expected in subsystem windows. However if you click it from the explorer or run it in visual studio it will hang when it tries to output. I don't know enough win32 to solve this issue but someone else might?

JesseRMeyer commented 4 years ago

Hmm, I've been doing some explorative win32 Zig development and haven't run into this. Are you running Windows 10? What Visual Studio version is installed? Can you provide a sample Zig program along with your build flags?

Rageoholic commented 4 years ago

Visual studio community 2019 v 16.3.9 zig build-exe hang.zig --subsystem windows

const std = @import("std");

pub fn main() void {
    std.debug.warn("Hello world\n", .{});
}
Sobeston commented 4 years ago

This is what you get for specifying subsystem windows - I'm not sure what you were expecting here. https://docs.microsoft.com/en-us/cpp/build/reference/subsystem-specify-subsystem?view=vs-2019 If you want a console (and don't want to spawn one yourself), the default subsystem console is what you need.

With that being said, the process hangs because the process calls NtWaitForKeyedEvent and waits for something that does not happen.

When you run your code from within a preexisting terminal perhaps the process receives this event (still failing to print, albeit silently), allowing the process to continue. I'd have to look into how to give a process a console when I'm debugging it to be sure.

I recently encountered this when working with writing a dll; the loaded dll panicked due to a nul pointer but instead of the process crashing, the dll just started hanging due to the default panic handler attempting to print.

I think zig could better handle things here.

Rageoholic commented 4 years ago

I was just expecting the output to go into the void, not for it to hang

JesseRMeyer commented 4 years ago

This is what you get for specifying subsystem windows - I'm not sure what you were expecting here. https://docs.microsoft.com/en-us/cpp/build/reference/subsystem-specify-subsystem?view=vs-2019

Where in that document does it suggest that there is no pipe to stderr for applications using the windows subsystem?

Sobeston commented 4 years ago

This is what you get for specifying subsystem windows - I'm not sure what you were expecting here. https://docs.microsoft.com/en-us/cpp/build/reference/subsystem-specify-subsystem?view=vs-2019

Where in that document does it suggest that there is no pipe to stderr for applications using the windows subsystem?

MS docs can be a bit all over the place, sorry - let me clarify. In console, the process can make a new console. In windows, the process will not make a new console, but also will not have any stderr and stdout pipes. It will expect you to create your own console, or access a pre-existing one for these.

A cheap workaround that will allow you to do what you want:

const std = @import("std");
usingnamespace std.os.windows;

extern "kernel32" fn AttachConsole(dwProcessId: DWORD) callconv(.Stdcall) BOOL;

pub fn main() !void {
    const ATTACH_PARENT_PROCESS: DWORD = 0xFFFFFFFF; //DWORD(-1)
    if (AttachConsole(ATTACH_PARENT_PROCESS) == 0) return error.CouldntAttachConsole;

    std.debug.warn("stderr :)\n", .{});

    const stdout = &std.io.getStdOut().outStream().stream;
    try stdout.print("stdout :)\n", .{});
}

This will allow the process to use the console from its parent process (be it cmd, powershell, etc). I believe the console is freed upon process exit, so FreeConsole() upon exit shouldn't be necessary. This isn't recommended as your process doesn't own the console, and the parent process may continue to manipulate the console - they both control it at once. It also has the side effect of needing to press enter right before process termination (weird, right?). That being said - probably fine for debugging.

I'm not sure how zig should handle this (no pun intended). Perhaps an easy way to redirect @panic and std.debug.warn to a different outstream like that of a file would be a good solution here. Maybe a runtime panic should happen if a pipe isn't present for printing... but I think @panic would end up calling itself as it also cannot print. Hope this helps

Sobeston commented 4 years ago
const std = @import("std");
usingnamespace std.os.windows;

fn setStdStream(stream: enum{in, out, err}, file: std.fs.File) !void {
    const f = struct {
        extern "kernel32" fn SetStdHandle(nStdHandle: DWORD, hHandle: HANDLE) callconv(.Stdcall) BOOL;
    };

    //TODO - better errors
    if (f.SetStdHandle( 
        @as(
            DWORD,
            switch(stream) {
                .in => STD_INPUT_HANDLE,
                .out => STD_OUTPUT_HANDLE,
                .err => STD_ERROR_HANDLE
            }
        ),
        file.handle,
    ) == 0) return error.CouldntSetStdHandle;
}

pub fn main() !void {
    try setStdStream(.out, try std.fs.cwd().createFile("stdout.txt", .{.truncate = true}));
    try setStdStream(.err, try std.fs.cwd().createFile("stderr.txt", .{.truncate = true}));
    try setStdStream(.in, try std.fs.cwd().createFile("stdin.txt", .{.truncate = true}));

    const stdout = &std.io.getStdOut().outStream().stream;
    try stdout.print("hello, world!\n", .{});
}

A much better solution for everyone.

andrewrk commented 4 years ago

Note that in zig we have the ability to detect at comptime that the subsystem is windows. So std.io.getStdOut() could return a no-op stream and give the expected behavior that the output goes nowhere.

andrewrk commented 4 years ago

Also related: #2586

andrewrk commented 4 years ago

This looks like it might be a deadlock due to std.debug.warn recursively calling itself due to unexpectedError:

[External Code]
test.exe!std.mutex.Mutex::std.mutex.Mutex.acquireSlow(std.mutex.Held * self, std.mutex.Mutex *) Line 128
    at C:\msys64\home\andy\dev\zig\lib\std\mutex.zig(128)
test.exe!std.mutex.Mutex::std.mutex.Mutex.acquire(std.mutex.Held * self, std.mutex.Mutex *) Line 100
    at C:\msys64\home\andy\dev\zig\lib\std\mutex.zig(100)
test.exe!std.debug.warn(std.os.windows.struct:1204:69 * args) Line 64
    at C:\msys64\home\andy\dev\zig\lib\std\debug.zig(64)
test.exe!std.os.windows.unexpectedError(std.builtin.StackTrace * err, std.os.windows.win32error.Win32Error) Line 1205
    at C:\msys64\home\andy\dev\zig\lib\std\os\windows.zig(1205)
test.exe!std.os.windows.WriteFile(std.os.windows.WriteFileError!usize * handle, std.builtin.StackTrace * bytes, c_void * offset, []u8 *) Line 486
    at C:\msys64\home\andy\dev\zig\lib\std\os\windows.zig(486)
test.exe!std.os.write(std.os.WriteError!usize * fd, std.builtin.StackTrace * bytes, c_void *) Line 533
    at C:\msys64\home\andy\dev\zig\lib\std\os.zig(533)
test.exe!std.fs.file.File::std.fs.file.File.write(std.os.WriteError!usize * self, std.builtin.StackTrace * bytes, std.fs.file.File *) Line 336
    at C:\msys64\home\andy\dev\zig\lib\std\fs\file.zig(336)
test.exe!std.fs.file.OutStream::std.fs.file.OutStream.writeFn(std.os.WriteError!usize * out_stream, std.builtin.StackTrace * bytes, std.io.out_stream.OutStream(std.os.WriteError) *) Line 549
    at C:\msys64\home\andy\dev\zig\lib\std\fs\file.zig(549)
test.exe!std.io.out_stream.OutStream(std.os.WriteError)::std.io.out_stream.OutStream(std.os.WriteError).writeOnce(std.os.WriteError!usize * self, std.builtin.StackTrace * bytes, std.io.out_stream.OutStream(std.os.WriteError) *) Line 30
    at C:\msys64\home\andy\dev\zig\lib\std\io\out_stream.zig(30)
test.exe!std.io.out_stream.OutStream(std.os.WriteError)::std.io.out_stream.OutStream(std.os.WriteError).write(std.builtin.StackTrace * self, std.io.out_stream.OutStream(std.os.WriteError) * bytes, []u8 *) Line 37
    at C:\msys64\home\andy\dev\zig\lib\std\io\out_stream.zig(37)
test.exe!std.fmt.formatText(std.builtin.StackTrace * bytes, []u8 * context, std.fmt.FormatOptions *) Line 593
    at C:\msys64\home\andy\dev\zig\lib\std\fmt.zig(593)
test.exe!std.fmt.formatType(std.builtin.StackTrace * value, []u8 * options, std.fmt.FormatOptions * context, std.io.out_stream.OutStream(std.os.WriteError) * max_depth, unsigned __int64) Line 459
    at C:\msys64\home\andy\dev\zig\lib\std\fmt.zig(459)
test.exe!std.fmt.format(std.builtin.StackTrace * context, std.io.out_stream.OutStream(std.os.WriteError) * args, struct:3:29 *) Line 184
    at C:\msys64\home\andy\dev\zig\lib\std\fmt.zig(184)
test.exe!std.io.out_stream.OutStream(std.os.WriteError)::std.io.out_stream.OutStream(std.os.WriteError).print(std.builtin.StackTrace * self, std.io.out_stream.OutStream(std.os.WriteError) * args, struct:3:29 *) Line 42
    at C:\msys64\home\andy\dev\zig\lib\std\io\out_stream.zig(42)
test.exe!std.debug.warn(struct:3:29 * args) Line 65
    at C:\msys64\home\andy\dev\zig\lib\std\debug.zig(65)
test.exe!panic([]u8 * message, std.builtin.StackTrace * stack_trace) Line 3
    at C:\msys64\home\andy\dev\zig\build\zig-cache\o\eTDq1PqDsX6D5R21dN2-H0ew7iD-Zu9JxerUeYu1Ccicg8YAJWxRasIH9Bbpf9B9\source.zig(3)
test.exe!main() Line 12
    at C:\msys64\home\andy\dev\zig\build\zig-cache\o\eTDq1PqDsX6D5R21dN2-H0ew7iD-Zu9JxerUeYu1Ccicg8YAJWxRasIH9Bbpf9B9\source.zig(12)
[Inline Frame] test.exe!std.start.callMain() Line 246
    at C:\msys64\home\andy\dev\zig\lib\std\start.zig(246)
[Inline Frame] test.exe!std.start.initEventLoopAndCallMain() Line 228
    at C:\msys64\home\andy\dev\zig\lib\std\start.zig(228)
test.exe!std.start.WinMainCRTStartup() Line 131
    at C:\msys64\home\andy\dev\zig\lib\std\start.zig(131)
[External Code]

The fix to this might be as simple as adding a new error to the switch case of WriteFile (you can find the file path and line number in the above stack trace)

strangebug commented 4 years ago

The recursion is caused by windows.unexpectedError which calls std.dbg.warn (see code). The call to GetLastError inside the function WriteFile returns an INVALID_HANDLE error which has no specific handler (see code).

I think we would lose valuable info for other error cases if we just add a new case to the switch. But as far as I understand, creating a no-op stream would require 1) a no-op File or 2) creating a real File with an invalid handle.

Personally not a big fan of option 2) but I don't think 1) is doable (at least at the moment) zig don't have support interface or trait. But I may be missing something, I am still new to zig.

strangebug commented 4 years ago

For future reference

The idea to have some sort of NoOpOutStream swappable with File.OutStream is not applicable (or at least without breaking changes).

We would want something that looks like the following

pub const OutStream = io.OutStream(File, WriteError, write); // Existing declaration
pub const NoOpStream = io.OutStream(File, WriteError, noOpWrite); // Potential solution

But the resulting type won't be assignable to std.fs.File.OutStream because it requires a compile time known function.

pub fn OutStream(
    comptime Context: type,
    comptime WriteError: type,
    comptime writeFn: fn (context: Context, bytes: []const u8) WriteError!usize,
) type {
    // Omitted
}
jdpage commented 4 years ago

I ran into this the other day, and solved it by having my program do the following at startup:

  1. Attempt to AttachConsole() to the parent process' console.
  2. If that fails, open the NUL device, and point the standard streams at it.

If the application is started by double-clicking, stderr output discarded. If it's started from inside a command prompt (e.g. as a zig build step), then the warnings are visible.

Additionally, the application can later attempt to AllocConsole() if it likes, and subsequent warnings will show up there automatically as the standard streams are replaced. (Unless the AttachConsole succeeded, but the application can always detach from it if it wants its own console.)

I can whip up a PR if this seems like a good solution.