-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
internal/poll: Reading data piped through os.Stdin hangs on Windows version #22024
Comments
Probably has something to do with the poller changes, but I don't know what. |
@jakans your program works with Windows type command:
I do not have curl program installed on my computer to test your scenario. How do I install it? What are you trying to do? If you just want to download some file from the Internet, Go standard library have plenty of tools to do that. Alex |
MSYS (with separately downloaded cURL)
MSYS2 http://nurmi-labs.blogspot.com/2016/11/git.html
|
Does it hang also, if you run it from cmd.exe? |
the content of #22024 (comment) indicates NO outout from curl 7.50.2 in both examples from that comment the Windows Console had been utilised for what its worth here is MSYS2's mintty.exe running a C Shell
You don't mention what cURL version you are using, or if you've run your cURL command without the pipe. |
@forskning thanks for testing. Looks like "go run pipehangs.go" does not hang in either of your scenarios. I wonder why curl 7.50.2 does not output any data. What happens if you redirect curl output to a file? Do you get anything written to the file? Thank you Alex |
@alexbrainman I should have looked into that prior to adding the MSYS content http://nurmi-labs.blogspot.com/2015/11/bcc55.html It was Dirk Paehl's cURL 7.50.2 (Download WITH SUPPORT SSL) I used for a Borland/Dmake compile of perl-5.10.1.tar.gz, the CPAN setup, and, as those were http addresses, I never added the SSL support files. Perl/perl5@378eeda#diff-65539b463d4890c68be9c1e3de589c4d You can read about the events which led up to the "The Borland Chainsaw Massacre" here: http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2011-09/msg00034.html |
Unable to reproduce with various versions of curl 7.55.1. http://www.paehl.com/open_source/?CURL_7.55.1
No crash, instant return
|
Does your curl command when run without the pipe output the xml to the console? |
Simply speaking you curl.exe was broken. Cool. Thank you for explaining.
Thank you @as for trying. Alex |
moot point curl_750_2_ssl (without adding the SSL support files) returns output for http not https
"That they be not forced to sue the lawe, wrapped with so infinite crickes and moot poyntes." Laurence Humphrey - 1563 |
The problem was discovered by users of Entrez Direct (https://www.ncbi.nlm.nih.gov/books/NBK179288/) who had installed the latest release on their PCs. While most of EDirect is implemented as a Perl wrapper to NCBI's URL-based Entrez Utilities (https://www.ncbi.nlm.nih.gov/books/NBK25501/), the xtract component is written in Go, and it failed to read pipes from the upstream Perl steps. I quickly reverted to the Go 1.8 xtract binary for CYGWIN (Mac and Linux versions are fine under Go 1.9), and then isolated the problem and created the minimal pipehangs.go program for debugging. Entrez Direct provides a simple way for non-programmers to do sophisticated data mining in PubMed and other interconnected NCBI databases. The target audience includes scientists, medical librarians, bibliometric researchers, and academic administrators, as well as computational biologists who are looking for an easy way to get specific information from our databases. Users supply query details in command-line arguments. Separate steps in the query are connected by Unix pipes. Everything else is handled behind the scenes, with no additional coding required. The sample ad hoc EDirect query shown below searches for journal articles in PubMed and retrieves the records in a defined XML form. The XML data set is piped to xtract, which limits results to journals published in the U.S. and then visits each author, printing one author name per line. This output is piped to a standard script that produces a frequency table of publications per author: esearch -db pubmed -query "rattlesnake phospholipase" | 9 Wells MA To answer the earlier question, curl.exe --version produces: |
The latest xtract.go source code now has an undocumented -echo command to help with debugging. xtract -sample will send a small sample XML file to stdout. xtract -echo will read from stdin, using the same method that is hanging when reading from a pipe. To obtain the source code without doing the full EDirect installation you can run: ftp ftp://ftp.ncbi.nlm.nih.gov/entrez/entrezdirect/edirect.tar.gz To build the PC binary using a Go 1.9 compiler on any platform, run: cd edirect On a PC under CYGWIN, the following will print a sample XML record: ./testxtract -sample Redirecting stdout will save it to a file: ./testxtract -sample > sample.xml Piping it to another instance of testxtract and reading with the -echo command will fail: ./testxtract -sample | ./testxtract -echo However, redirecting stdin from an existing file will work: ./testxtract -echo < sample.xml The actual code being tested is excerpted here:
|
It would be nice to see the dataflow through the pipe. Can you run your pipeline through pv? If you're using Windows you can use the implementation here:
|
I did that.
I did that.
I don't have CYGWIN, I used cmd.exe program.
I did that. It worked.
I did that. It worked.
I did that. It worked.
I did that. It worked.
I think your code has a bug. When in.Read(buffr) returns with n == 0 you break out of the read loop. But there could be more data coming. The Reader documentation says https://golang.org/pkg/io/#Reader about that: "... Callers should treat a return of 0 and nil as indicating that nothing happened; in particular it does not indicate EOF. ...". Perhaps your CYGWIN shell program (unlike my cmd.exe) inserts 0 length writes into the pipe between 2 programs. Alex |
In the event anyone decides to pursue testing the Cygnus port. A search of the current curl and perl implementations follows. https://cygwin.com/cgi-bin2/package-grep.cgi curl-7.55.1-1 Pour faire l'Archiviste il faut être un homme intelligent. Un homme intelligent ne fait pas l'Archiviste. |
I've modified the -echo code to report if "n == 0" without breaking out of the loop, but the test had been added merely to suppress the EOF error message. The actual function that calls Read only looks for "err != nil" to determine end of data. To further investigate, I added a -read command to specifically test the actual read block function. These are in the xtract.go that is now on the ftp site. The relevant code section is:
I asked my colleague with a PC to try: testxtract -sample | testxtract -echo and: testxtract -sample | testxtract -read and he got wildly inconsistent results. Sometimes it would hang, sometimes it wouldn't. Inserting pv in between would either hang, print one "pv:" line and then the expected output, or print "pv:" lines forever. My expert-of-last-resort colleague got involved, and quickly found that he could force consistent hangs by replacing: testxtract -sample in the above commands with: (sleep 1; testxtract -sample) That was immediately confirmed by the other colleague. Do you know of any change between Go 1.8 and Go 1.9 that might explain this sort of timing dependency with pipes on the PC? |
|
Just because
Above is a cautious way to do this, i.e., don't process the bytes if a real error occurred. The |
I had noticed this in the documentation, but in my early tests err != nil always came with n == 0. To be robust against future implementation changes, I've modified the code, but I still prefer to err on the side of caution and ignore bytes that come with a real error. It will now print any non-EOF error message to Stderr, at least making the user aware that something is amiss. The -echo test code was also changed, with the relevant section shown below:
The source code on the ftp site has been updated, but I'll allow some time for in-house testing before making a new release of the precompiled binaries. Thanks for the advice. |
Not a problem, but just FYI my example and yours are semantically similar - they don't handle data if there's a real error. This is because p[:n], when n==0 is a valid zero-length slice. The disadvantage is that it doesn't guard against non-conforming implementations of io.Reader that return -1. Below I used
|
@jakans and @as, I think, you are still wrong with your code. As per io.Reader documentation: "... When Read encounters an error or end-of-file condition after successfully reading n > 0 bytes, it returns the number of bytes read. ...". So if Read returns n > 0, you must process p[:n] bytes regardless what the err is. And you don't, because you break out of for loop. @jakans are you still having a problem? If yes, I take it you can reproduce it with a Go program (without curl). Is that correct? If so, can you, show us that program, how you run it, what the program outputs, and why you think the output is wrong. If we need CYGWIN to reproduce, tell us where to get it and how to install it. Thank you. Alex |
I agree it is incorrect, but a mild amount of APIs outside of stdlib return mangled data on a non-EOF error. I would love for this to work everywhere, it a lot easier on the eyes too.
|
presuming from #22024 (comment) the hardware utilised is 64bit https://cygwin.com/install.html Installing and Updating Cygwin for 64-bit versions of Windows |
That is the CYGWIN address I was about to pass along. The current xtract.go source code has the modified -echo command that can handle n == 0 blocks. You can download it by running: ftp ftp://ftp.ncbi.nlm.nih.gov/entrez/entrezdirect/edirect.tar.gz and build the PC executable with: env GOOS=windows GOARCH=386 go build -o testxtract -v xtract.go You should be able to reproduce the hanging problem when built under Go 1.9 by running: (sleep 1; testxtract -sample) | testxtract -echo and: (sleep 1; testxtract -sample) | pv | testxtract -echo For reference, here is the current -echo source code:
|
If CYGWIN is needed to reproduce, beyond the minimal base packages, what added packages would be relevant? Not pertinent to this thread, but IIRC historically bi-directional named pipes have been problematic on CYGWIN. |
I'm told that the base system should be sufficient. |
Thanks for testing it, and of course for all the work to get to this point. Since this fixes the problem, are you and the other developers satisfied with the change, and will be in 1.9.2 and future versions? |
No problem. This issue did not affect me personally (yet), I just wanted to help see what the problem was and fix it. I am happy that @alexbrainman got the poller to behave, and time will tell if the change is satisfying. |
https://groups.google.com/forum/#!topic/golang-dev/naQlgeWVtvA @jakans I believe Go 1.9.2 is the subject of the above discussion. |
I satisfy myself that https://go-review.googlesource.com/#/c/go/+/69871 change is not doing bad things. I wouldn't know if it fixes your problem, because I could not reproduce it. But @as checked that. So I will take his word.
I suggested that change for go1.9.2 on Alex |
|
It looks like if we are going to fix this for Go 1.9.2, the patch will apply cleanly only if we first also fix #22149, which it looks like we should fix anyway. |
Change https://golang.org/cl/70990 mentions this issue: |
Since I still have the environment to reproduce this issue, I can re-test any intermediate versions if necessary. |
CL 53530 OK for Go 1.9.2. |
…ficationModes for sockets CL 36799 made SetFileCompletionNotificationModes to be called for file handles. I don't think it is correct. Revert that change. Fixes #22024 Fixes #22207 Change-Id: I26260e8a727131cffbf60958d79eca2457495554 Reviewed-on: https://go-review.googlesource.com/69871 Reviewed-by: Ian Lance Taylor <[email protected]> Reviewed-on: https://go-review.googlesource.com/70990 Run-TryBot: Russ Cox <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Alex Brainman <[email protected]>
go1.9.2 has been packaged and includes: The release is posted at golang.org/dl. — golang.org/x/build/cmd/releasebot, Oct 26 21:09:18 UTC |
tomnomnom/gron#32 occurs on Go 1.9 but not on 1.10. |
Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (
go version
)?go1.9 darwin/amd64, but cross-compiling for PC/CYGWIN
Does this issue reproduce with the latest release?
Yes, issue is only on go1.9 and Windows, go1.8 is fine on all platforms
What operating system and processor architecture are you using (
go env
)?Windows under CYGWIN. (Go compiler is not installed on target machines.)
What did you do?
curl -s "https://eutils.ncbi.nlm.nih.gov/entrez/eutils/esummary.fcgi?db=pubmed&id=26422376&version=2.0" |
go run pipehangs.go
If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.
Source code of pipehangs.go is inserted here:
package main
import (
"fmt"
"io/ioutil"
"os"
)
func main() {
}
File attached, with .txt suffix added to get past data format test:
pipehangs.go.txt
What did you expect to see?
n: 3910
What did you see instead?
Program hangs.
It's fine if you pipe a file:
cat testfile.txt | go run pipehangs.go
or redirect stdin:
go run pipehangs.go < testfile.txt
Only reading a network result under Windows and go1.9 causes it to hang.
The text was updated successfully, but these errors were encountered: