os/signal: TestCtrlBreak fails on windows/386

[alexbrainman]

TestCtrlBreak fails on windows/386

--- FAIL: TestCtrlBreak (2.21s)
signal_windows_test.go:101: Program exited with error: exit status -1073741510
FAIL
FAIL os/signal 2.280s

https://storage.googleapis.com/go-build-log/7dd1854f/windows-386-gce_0e29ac61.log

but succeeds on windows/amd64. I cannot reproduce it here. Strange.

The exit status is STATUS_CONTROL_C_EXIT. I can reproduce the failure, if I disable our C-break handler (comment out call to SetConsoleCtrlHandler in runtime).

Alex

[minux]

It's it possible that there is race between we send ^C to the process and the process installs its control-break handler?

[alexbrainman]

I was thinking about the race (in our test). But I don't see how. Parent process waits for 1 second. Surely child should be finished installing the handler. Perhaps I can change the test for parent to read child's output for some "ready" message, but I am grasping for straws here. It would be nice if I can reproduce it here. It also does not break on windows/amd64. Why?

[minux]

could we skip the test for now? I want to do a final trybot run on my external linking
CLs and submit them.

I've sent CL 7971 to skip the test on windows/386.

[alexbrainman]

I lied. I can reproduce it here. I did git bisect and that is what I find:

b115c35 is the first bad commit
commit b115c35
Author: Russ Cox rsc@golang.org
Date: Wed Mar 18 17:26:36 2015 -0400

cmd/internal/gc: move cgen, regalloc, et al to portable code

This CL moves the bulk of the code that has been copy-and-pasted
since the initial 386 port back into a shared place, cutting 5 copies to 1.

The motivation here is not cleanup per se but instead to reduce the
cost of introducing changes in shared concepts like regalloc or general
expression evaluation. For example, a change after this one will
implement x.(*T) without a call into the runtime. This CL makes that
followup work 5x easier.

The single copy still has more special cases for architecture details
than I'd like, but having them called out explicitly like this at least
opens the door to generalizing the conditions and smoothing out
the distinctions in the future.

This is a LARGE CL. I started by trying to pull in one function at a time
in a sequence of CLs and it became clear that everything was so
interrelated that it had to be moved as a whole. Apologies for the size.

It is not clear how many more releases this code will matter for;
eventually it will be replaced by Keith's SSA work. But as noted above,
the deduplication was necessary to reduce the cost of working on
the current code while we have it.

Passes tests on amd64, 386, arm, and ppc64le.
Can build arm64 binaries but not tested there.
Being able to build binaries means it is probably very close.

Change-Id: I735977f04c0614f80215fb12966dfe9bbd1f5861
Reviewed-on: https://go-review.googlesource.com/7853
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

Alex

[mattn]

signal handling doesn't work?

package main

import (
    "log"
    "os"
    "os/signal"
    "time"
)

func main() {
    c := make(chan os.Signal, 10)
    signal.Notify(c)
    select {
    case s := <-c:
        log.Println("catch!")
        if s != os.Interrupt {
            log.Fatalf("Wrong signal received: got %q, want %q\n", s, os.Interrupt)
        }
    case <-time.After(3 * time.Second):
        log.Fatalf("Timeout waiting for Ctrl+Break\n")
    }
}

go build
ctrlbreak

And type CTRL-BREAK or CTRL-C, But catch! doesn't appear.

[mattn]

os1_windows.go

func ctrlhandler1(_type uint32) uint32 {
    var s uint32

    switch _type {
    case _CTRL_C_EVENT, _CTRL_BREAK_EVENT:
        s = _SIGINT
    default:
        return 0
    }

    if sigsend(s) {
        println(1) // ADD PRINTLN HERE
        return 1
    }
    exit(2) // SIGINT, SIGTERM, etc
    return 0
}

When added println in above, it works fine. But not, it doesn't work correctly.

[minux]

Interesting. That change is huge (+3935, -10636), but it should only affect code generation. If it's causing problems, then 8g should miscompile some code. Please try produce a minimal test case that 8g miscompiles. If I understand @mattn's comment correctly, 8g miscompiles ctrlhandler1.