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

os: readdirent regression on El Capitan #12865

Closed
Southern opened this issue Oct 7, 2015 · 22 comments
Closed

os: readdirent regression on El Capitan #12865

Southern opened this issue Oct 7, 2015 · 22 comments

Comments

@Southern
Copy link

@Southern Southern commented Oct 7, 2015

#8423 is, once again, occurring on El Capitan with Go 1.5.

@Southern Southern changed the title File I/O regression on El Capitan readdirent regression on El Capitan Oct 7, 2015
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 7, 2015

Can you show us some code and show us what happens? Thanks.

@ianlancetaylor ianlancetaylor changed the title readdirent regression on El Capitan os: readdirent regression on El Capitan Oct 7, 2015
@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Oct 7, 2015
@Southern

This comment has been minimized.

Copy link
Author

@Southern Southern commented Oct 7, 2015

My apologies. I meant to grab the code and update the original post.

This is the code that I ran into the error with:

    p, e := parser.ParseDir(o.set, path, nil, parser.AllErrors)
    if e != nil {
        return nil, e
    }

o is an instance of:

type Ob struct {
    set *token.FileSet
}

readdirent is returning the same "invalid argument" that it was when I reported this for Yosemite:

Error: readdirent: invalid argument
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 7, 2015

If you apply the patch in https://codereview.appspot.com/119530044/diff/80001/src/pkg/syscall/syscall_bsd.go to the file syscall/syscall_bsd.go, does it fix the problem? Do all the tests pass with that patch applied?

@cespare

This comment has been minimized.

Copy link
Contributor

@cespare cespare commented Oct 7, 2015

@Southern can you show a runnable example to demonstrate the problem? #8423 says that os.RemoveAll doesn't work on an ioutil.TempDir, but that is working fine for me on El Capitan + Go 1.5:

http://play.golang.org/p/bCUsg-u1kS

Or are you saying there's a problem with go/parser specifically? (In that case, what's the relation to #8423?)

@Southern Southern closed this Oct 7, 2015
@Southern Southern reopened this Oct 7, 2015
@Southern

This comment has been minimized.

Copy link
Author

@Southern Southern commented Oct 7, 2015

You would think I'd know where that close issue button is by now.

@cespare I haven't tried os.RemoveAll yet. This is just the first case that I ran into this happening, and my assumption was that it's a regression of #8423.

@Southern

This comment has been minimized.

Copy link
Author

@Southern Southern commented Oct 7, 2015

@ianlancetaylor that patch does work, so this is indeed a regression.

@Southern

This comment has been minimized.

Copy link
Author

@Southern Southern commented Oct 7, 2015

@cespare do you know of a pastebin service that runs on OS X? I'm pretty sure the playground is on a *NIX box. This seems to be an OS X only bug.

@cespare

This comment has been minimized.

Copy link
Contributor

@cespare cespare commented Oct 7, 2015

@Southern I pasted the code that I ran on my own El Capitan machine.

@Southern

This comment has been minimized.

Copy link
Author

@Southern Southern commented Oct 7, 2015

@cespare right, but the playground itself is running on *NIX... so any example that I give you is going to pass there since this isn't a problem with *NIX in general. That's why I was asking if you know of anywhere that can run Go (or is hosting the playground) on OS X.

@cespare

This comment has been minimized.

Copy link
Contributor

@cespare cespare commented Oct 7, 2015

@Southern I don't know of such a hosting service.

play.golang.org is the typical way we share code snippets, even if the reproduction of the bug requires some other OS.

@Southern

This comment has been minimized.

Copy link
Author

@Southern Southern commented Oct 7, 2015

@cespare true story. You asked for a runnable example, so I assumed you meant "runnable on the playground" since you posted a playground link.

Here's the code on the playground: http://play.golang.org/p/nwwOfAQHCC

@cespare

This comment has been minimized.

Copy link
Contributor

@cespare cespare commented Oct 7, 2015

@Southern I would like it to be runnable on the playground :)

What would be helpful is a snippet of code that runs without a problem on the playground. Then, when I run the same code on my El Capitan machine, I would see the failure case.

Your example (a) doesn't do anything with the error (print it, etc), and (b) tries to open a file that doesn't exist, so by itself it still doesn't show me the problem you're having.

@Southern

This comment has been minimized.

Copy link
Author

@Southern Southern commented Oct 7, 2015

@cespare the error isn't coming from the example code itself. The error is coming from lower than the ReadDir function. I wasn't printing the error in my original code. I'm also not aware of the paths that are available in the playground, so I threw in a random directory to read from. Surely a one line edit isn't that serious.

@cespare

This comment has been minimized.

Copy link
Contributor

@cespare cespare commented Oct 7, 2015

@cespare the error isn't coming from the code itself. The error is coming from lower than the ReadDir function. I wasn't printing the error in my original code.

So how do you know there's an error? Is the code panicing?

I'm also not aware of the paths that are available in the playground, so I threw in a random directory to read from. Surely a one line edit isn't that serious.

You can e.g. make a tempdir on the playground and populate it.

Or even if your reproducer has

// mkdir /some/path before running this

then I would know what to do. FWIW, I did try changing your code to point to a dir that exists on my El Capitan machine and still didn't get any errors.


To back up one step: a good bug report has:

  1. Execution environment (OS, Go version)
  2. What you did (best is short, runnable code)
  3. What you expected to happen
  4. What actually happened

I understand that (1) is Go 1.5 and Mac OS X El Capitan, but I still don't have a clear understanding of 2, 3, and 4.

I'm not trying to be pedantic or obtuse here -- I really don't know how I can make the problem happen on my machine. Perhaps it's clear to someone else with an El Capitan box handy?

@Southern

This comment has been minimized.

Copy link
Author

@Southern Southern commented Oct 7, 2015

@cespare that's literally all of the code that I'm using to reproduce this, and it consistently fails without the patch for #8423. I really have no idea why it's not reproducible on your side.

All I'm trying to do is parse a directory of Go source files with parser.ReadDir.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Nov 5, 2015

@Southern, can you please try http://swtch.com/~rsc/readdirbug.c? That's the demo program I filed with Apple regarding #8423. If it fails, can you send both the output of that program and the sw_vers command? Thanks.

@Southern

This comment has been minimized.

Copy link
Author

@Southern Southern commented Nov 5, 2015

@rsc Hm. Looks like that example passed with no problem.

 ❱ ./readdirbug 
mkdir /tmp/readdirbug
create /tmp/readdirbug/file1

opendir /tmp/readdirbug
readdir: found .
readdir: found ..
readdir: found file1
readdir: EOF
readdir: EOF
readdir: EOF
closedir

opendir /tmp/readdirbug
readdir: found .
readdir: found ..
readdir: found file1
readdir: EOF
rm /tmp/readdirbug/file1
readdir: EOF
readdir: EOF
closedir
ok - everything worked
@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Nov 6, 2015

OK, well what does sw_vers print for you anyway? I will check on my El Capitan systems at home too but it would be good to know if we have the same software.

@Southern

This comment has been minimized.

Copy link
Author

@Southern Southern commented Nov 6, 2015

ProductName:    Mac OS X
ProductVersion: 10.11
BuildVersion:   15A284
@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Dec 11, 2015

@Southern, it took me a while to get a Mac updated to El Capitan, but I have. Mine says:

ProductName:    Mac OS X
ProductVersion: 10.11.2
BuildVersion:   15C50

Running my C test program does not show a problem (nor did it on your machine).

I took your program and replaced "/some/path/to/read" with os.Args[1], so I could name the path on the command line and try various directories. I was unable to make the program fail with any directory I tried.

A few questions then:

  1. If you update to 10.11.2, do you still see the problem?
  2. Do you see the problem on every ParseDir, or do you have other concurrent modifications to the directory (such as removal of files)?
  3. Using the test program you posted, can you make it fail when reading any of the directories in the Go standard library? If so, which ones?
  4. If not, can you give us some other self-contained way to reproduce the problem?

Without a way to reproduce the problem, I'm very reluctant to reapply that patch. It was a very big hammer, not one we really wanted to use. Without being able to poke at the problem ourselves, I'm not inclined to use it again.

If you can't show us how to reproduce it, we can leave things as they are and hope that someone else runs into the problem and has more luck reproducing it.

Thanks.

@Southern

This comment has been minimized.

Copy link
Author

@Southern Southern commented Dec 15, 2015

I haven't been able to reproduce it again. Closing this.

@Southern Southern closed this Dec 15, 2015
@ncw

This comment has been minimized.

Copy link
Contributor

@ncw ncw commented May 30, 2016

Hmmm, I seem to be seeing this fairly regularly in my tests. Maybe a 50:50 chance that

go test -cpu=2 github.com/ncw/rclone/fs

will blow up with:

Failed to read directory: /var/folders/gw/_2jq29095y7b__wtby9dg_5h0000gn/T/rclone836042504: readdirent: invalid argument

You can see this in the travis logs here

It only happens when -cpus=2 and only on OS X - the tests run fine on Linux and Windows and clean under the race detector.

The tests do do concurrent directory listings.

Unfortunately I don't have an OS X machine, I only have access to OS X via travis which makes tracking down bugs rather tedious!

According to the travis docs these tests are running on a virtual machine running OS X 10.9.5 by default.

I told travis to use OS X 10.11 and that seemed to fix it...

That is about as much debugging as I'm going to do on this, not having an OS X machine, so I hope this is useful to someone!

@golang golang locked and limited conversation to collaborators May 30, 2017
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
6 participants
You can’t perform that action at this time.