Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fmt: Sometimes fmt.Scan failed to read integer on Windows #8825

Closed
gopherbot opened this issue Sep 27, 2014 · 18 comments
Closed

fmt: Sometimes fmt.Scan failed to read integer on Windows #8825

gopherbot opened this issue Sep 27, 2014 · 18 comments

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 27, 2014

by bupjae:

Before filing a bug, please check whether it has been fixed since the
latest release. Search the issue tracker and check that you're running the
latest version of Go:

Run "go version" and compare against
http://golang.org/doc/devel/release.html  If a newer version of Go exists,
install it and retry what you did to reproduce the problem.

Thanks.

What does 'go version' print?

go version go1.3.2 windows/amd64

What steps reproduce the problem?
If possible, include a link to a program on play.golang.org.

1. write test.go as http://play.golang.org/p/gc0njJ_GW2
2. "go run test.go" and input a integer from keyboard several times

What happened?

Sometimes fmt.Scan failed to read integer

What should have happened instead?

fmt.Scan should read integer successfully, always

Please provide any additional information below.

Screenshot attached.

Attachments:

  1. error.png (28278 bytes)
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 28, 2014

Comment 1:

I can't recreate this on GNU/Linux.  Can anybody recreate it on Windows?

Labels changed: added repo-main, os-windows.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Sep 29, 2014

Comment 2:

I can recreate it on Windows. Makes it easy if I change go command like that:
diff -r 9472d0e26c23 src/cmd/go/run.go
--- a/src/cmd/go/run.go Sun Sep 28 08:27:05 2014 -0700
+++ b/src/cmd/go/run.go Mon Sep 29 12:48:41 2014 +1000
@@ -10,6 +10,7 @@
        "os/exec"
        "runtime"
        "strings"
+       "time"
 )
 var execCmd []string // -exec flag, for run and test
@@ -63,6 +64,7 @@
 }
 func runRun(cmd *Command, args []string) {
+       time.Sleep(3*time.Second)
        raceInit()
        var b builder
        b.init()
and then type "1" before the time.Sleep above completes, followed by "enter key" after
long pause when the running program already started.
Alex
@ceh
Copy link
Contributor

@ceh ceh commented Oct 16, 2014

Comment 3:

Unable to reproduce on go version devel +ad9e191a5194 Wed Oct 15 17:54:04 2014 -0700
windows/amd64
Is there a minimal reproducible case?
@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Oct 17, 2014

Comment 4:

c.emil.hessman,
See my instructions in #2. Alternatively, you can do this:
C:\go\root\src\os\exec>hg diff
diff -r 3e5d0270f09e src/os/exec/exec_test.go
--- a/src/os/exec/exec_test.go  Thu Oct 16 15:08:49 2014 -0700
+++ b/src/os/exec/exec_test.go  Fri Oct 17 16:58:01 2014 +1100
@@ -720,8 +720,41 @@
                }
                fmt.Print(p)
                os.Exit(0)
+       case "alex":
+               alex()
        default:
                fmt.Fprintf(os.Stderr, "Unknown command %q\n", cmd)
                os.Exit(2)
        }
 }
+
+func alex() {
+       defer os.Exit(0)
+
+       fmt.Println("now press Enter key ...")
+       x := make([]byte, 10)
+       n, err := os.Stdin.Read(x)
+       x = x[:n]
+       fmt.Printf("n=%d err=%v\n", n, err)
+       fmt.Printf("x=%v\n", x)
+       fmt.Printf("x=[")
+       for _, c := range x {
+               fmt.Printf("%c ", c)
+       }
+       fmt.Println("]")
+}
+
+func TestAlex(t *testing.T) {
+       fmt.Println(`type "abc" quickly (you have 5 seconds) ...`)
+
+       time.Sleep(5 * time.Second)
+
+       p := helperCommand(t, "alex")
+       p.Stdin = os.Stdin
+       p.Stdout = os.Stdout
+       p.Stderr = os.Stderr
+       err := p.Run()
+       if err != nil {
+               t.Error(err)
+       }
+}
C:\go\root\src\os\exec>go test -c && exec.test.exe -test.v -test.run=Alex
=== RUN TestAlex
type "abc" quickly (you have 5 seconds) ...
now press Enter key ...
abc
n=5 err=<nil>
x=[101 120 101 99 46]
x=[e x e c . ]
--- PASS: TestAlex (8.56s)
PASS
You need to run test binary directly (run exec.test.exe, not "go test ..."). When asked
to type "abc" on keyboard, type it, then wait for "now press Enter key" prompt before
pressing Enter. As you can see from my output, our test reads "exec.", instead of "abc"
from keyboard.
I don't know what the problem is. Yet.
Alex
@gopherbot gopherbot added new labels Oct 17, 2014
@bradfitz bradfitz removed the new label Dec 18, 2014
@rsc rsc removed the os-windows label Apr 10, 2015
@rsc rsc added this to the Go1.5Maybe milestone Apr 10, 2015
@rsc rsc added OS-Windows and removed repo-main labels Apr 14, 2015
@robpike robpike removed their assignment Jun 2, 2015
@rsc
Copy link
Contributor

@rsc rsc commented Jun 29, 2015

I tried to write an OS-independent test for this but failed. Anyone see what I did wrong? It seems to me that if the problem is in \r handling, it should be possible to reproduce with an appropriate input. Seems too late for Go 1.5 in any event.

package main

import (
    "fmt"
    "log"
    "os"
)

func main() {
    var x int64
    r, w, err := os.Pipe()
    if err != nil {
        log.Fatal(err)
    }
    os.Stdin = r
    w.Write([]byte("1\r\n2\r\n"))
    n, err := fmt.Scanf("%d", &x)
    fmt.Println("return =", n, err)
    fmt.Println("x =", x)
}
@rsc rsc modified the milestones: Unplanned, Go1.5Maybe Jun 29, 2015
@rsc
Copy link
Contributor

@rsc rsc commented Jun 29, 2015

Maybe this is a dup of #8944, which I can reproduce portably.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 1, 2015

I still don't know what the problem is.

Maybe it is something to do with the fact that stdin handling whole lines only. My test starts typing the line before child starts and ends after child is started. Inserting pipe between parent and child's stdin fixes the problem:

diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go
index 3773963..fed35c2 100644
--- a/src/os/exec/exec_test.go
+++ b/src/os/exec/exec_test.go
@@ -760,8 +760,54 @@ func TestHelperProcess(*testing.T) {
        }
        fmt.Print(p)
        os.Exit(0)
+   case "alex":
+       alex()
    default:
        fmt.Fprintf(os.Stderr, "Unknown command %q\n", cmd)
        os.Exit(2)
    }
 }
+
+func alex() {
+   defer os.Exit(0)
+
+   fmt.Println("now press Enter key ...")
+   x := make([]byte, 10)
+   n, err := os.Stdin.Read(x)
+   x = x[:n]
+   fmt.Printf("n=%d err=%v\n", n, err)
+   fmt.Printf("x=%v\n", x)
+   fmt.Printf("x=[")
+   for _, c := range x {
+       fmt.Printf("%c ", c)
+   }
+   fmt.Println("]")
+}
+
+func TestAlex(t *testing.T) {
+   fmt.Println(`type "abc" quickly (you have 5 seconds) ...`)
+
+   time.Sleep(5 * time.Second)
+
+   p := helperCommand(t, "alex")
+
+   // p.Stdin = os.Stdin
+   childStdin, err := p.StdinPipe()
+   if err != nil {
+       t.Fatal(err)
+   }
+   defer childStdin.Close()
+   go func() {
+       _, err := io.Copy(childStdin, os.Stdin)
+       if err != nil {
+           t.Fatal(err)
+       }
+   }()
+
+   p.Stdout = os.Stdout
+   p.Stderr = os.Stderr
+   err = p.Run()
+   if err != nil {
+       t.Fatal(err)
+   }
+}

But that approach will not work in cmd/go - I think we want "go run ..." child reading from console, not from pipe.

Alex

@robpike
Copy link
Contributor

@robpike robpike commented Jul 1, 2015

I don't believe this is related to #8944. The issue in #8944 is specifically about how newlines work in Scanf. This is Scan, not Scanf, and newlines work differently in the two.

It seems to be confined to Windows, which argues against it being \r in particular. If it were, we could reproduce it with \r on other systems, but we cannot.

My guess is that this has to do with a difference in how Windows does console input but I doubt that is a helpful remark.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 2, 2015

My sentiment exactly @robpike

Alex

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Dec 15, 2016

I have built little program https://play.golang.org/p/W4f9GPsoID that demonstrates this. If I run it (and press "abc", when prompted, followed by Enter), I see this:

C:\>u:\test
type "abc" quickly (you have 5 seconds) ...
now press Enter key ...
abc
2016/12/15 17:20:58 child have "t\x00e\x00s", but want "abc\r\n"
2016/12/15 17:20:58 exit status 1

C:\>

But if I change program a little:

diff --git a/main.go b/main.go
index 3881bf6..0459d96 100644
--- a/main.go
+++ b/main.go
@@ -29,7 +29,7 @@ const testdata = "abc"
 func child(arg string) error {
 	fmt.Println("now press Enter key ...")
 	// TODO: change x size to 41 makes test pass. Why?
-	x := make([]byte, 40)
+	x := make([]byte, 41)
 	n, err := readStdin(x)
 	if err != nil {
 		return err

and run in the same way, I see

C:\>u:\test
type "abc" quickly (you have 5 seconds) ...
now press Enter key ...
abc

C:\>

I also built similar Pascal program https://play.golang.org/p/ipEm9iGnnE, and it behaves in the same way

C:\>c:\tmp\test
type something quickly (you have 5 seconds) ...
abc
ReadFile return 5 74 0 65 0 73
press a key
^C
C:\>

It even have same bug if I change buffer size from 40 to 41

C:\>c:\tmp\test
type something quickly (you have 5 seconds) ...
abc
ReadFile return 5 61 62 63 D A
press a key
^C
C:\>

So both programs are broken in exactly the same way. I am not even sure, if it is possible to enter characters into console buffer before program is started and expect all characters to be read by the program properly.

Alex

@tural-esger
Copy link

@tural-esger tural-esger commented Feb 6, 2018

this issue still exists in go 1.9.3

@mattn
Copy link
Member

@mattn mattn commented Feb 7, 2018

os.Stdin of child process should not be "console". But Go handle it as "console". (see newFile in os/file_windows.go) So child use ReadConsole instead of ReadFile. ReadConsole read 2 bytes for each input characters. Then, it result should contains 0x00.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Feb 7, 2018

this issue still exists in go 1.9.3

Yes. I do not know how to fix it, and no one else tried to fix it.

os.Stdin of child process should not be "console". But Go handle it as "console". (see newFile in os/file_windows.go) So child use ReadConsole instead of ReadFile. ReadConsole read 2 bytes for each input characters. Then, it result should contains 0x00.

Sorry @mattn but I don't understand you. Please try again.

Alex

@mattn
Copy link
Member

@mattn mattn commented Feb 7, 2018

@alexbrainman

C:\>c:\tmp\test
type something quickly (you have 5 seconds) ...
abc
ReadFile return 5 74 0 65 0 73

This is part of string test that you run. I compiled your code as named issue8825.go and run.

win7

readStdin returun "issue". This seems reading invalid memory.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Feb 8, 2018

readStdin returun "issue". This seems reading invalid memory.

Yes, there is a problem here. And I cannot explain it.

Alex

@mattn
Copy link
Member

@mattn mattn commented Feb 9, 2018

@alexbrainman This seems not be caused by exec.Command or pipe. Below is more smaller code to reproduce.

package main

import (
	"fmt"
	"os"
	"syscall"
	"time"
)

func readStdin(b []byte) (int, error) {
	h, err := syscall.GetStdHandle(syscall.STD_INPUT_HANDLE)
	if err != nil {
		return 0, err
	}

	var done uint32
	err = syscall.ReadFile(syscall.Handle(h), b, &done, nil)
	if err != nil {
		return 0, err
	}
	return int(done), nil
}

const testdata = "abc"

func test() int {
	fmt.Println("now press Enter key ...")
	// TODO: change x size to 41 makes test pass. Why?
	x := make([]byte, 40)
	n, err := readStdin(x)
	if err != nil {
		fmt.Fprintf(os.Stderr, "readStdin fail %v\n", err)
		return -1
	}
	x = x[:n]
	have := string(x)
	want := testdata + "\r\n"
	if want != have {
		fmt.Fprintf(os.Stderr, "have %q, but want %q\n", have, want)
		return -1
	}
	return 0
}

func main() {
	fmt.Printf("type %q quickly (you have 5 seconds) ...\n", testdata)
	time.Sleep(5 * time.Second)
	os.Exit(test())
}

And C code can reproduce this.

#include <windows.h>
#include <stdio.h>

#define testdata "abc"

int
main(int argc, char* argv[]) {
  char b[40];
  DWORD done = 0;
  int i;

  printf("type %s quickly (you have 5 seconds) ...\n", testdata);
  Sleep(5000);
  puts("now press Enter key ...");

  HANDLE h = GetStdHandle(STD_INPUT_HANDLE);
  if (!ReadFile(h, b, sizeof(b), &done, NULL)) {
	fprintf(stderr, "read error %d\n", GetLastError());
	return -1;
  }
  b[done] = 0;
  if (strcmp(testdata, b) != 0) {
    for (i = 0; i < done; i++) {
      printf("%02x ", b[i]);
    }
    puts("");
  }
  return 0;
}
type abc quickly (you have 5 seconds) ...
now press Enter key ...
abc
61 00 2e 00 65

So, I'm thinking this is not a bug of Go, and this issue is closable.

@robpike
Copy link
Contributor

@robpike robpike commented Feb 9, 2018

Closing as this is a Windows problem unrelated to Go.

@robpike robpike closed this Feb 9, 2018
@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Feb 10, 2018

I'm thinking this is not a bug of Go

I am thinking the same.

Closing as this is a Windows problem unrelated to Go.

I suspect so.

But before we give up on this, I will ask @jstarks - can you investigate what I described here #8825 (comment) ? Where is the problem?

Thank you.

Alex

@golang golang locked and limited conversation to collaborators Feb 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants
You can’t perform that action at this time.