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: Chdir is not reentrant #27658

Closed
blissnd opened this issue Sep 13, 2018 · 9 comments
Closed

os: Chdir is not reentrant #27658

blissnd opened this issue Sep 13, 2018 · 9 comments

Comments

@blissnd
Copy link

blissnd commented Sep 13, 2018

Reproduction and description of bug:

https://github.com/blissnd/BUG_IN_GOLANG

Using LockOSThread() makes no difference to the issue as shown here:

https://github.com/blissnd/BUG_IN_GOLANG/tree/workaround?files=1

Concurrency code should be reentrant and should not keep state between threads, especially when their is no access to shared memory or race conditions.

@ianlancetaylor ianlancetaylor changed the title Bug report: Concurrency code with system/shell calls is not reentrant os: Chdir is not reentrant Sep 13, 2018
@ianlancetaylor
Copy link
Contributor

In short, the bug report is that if two different goroutines call os.Chdir concurrently, it is unpredictable which will take effect.

That is true. os.Chdir is a process-wide attribute, not a per-goroutine or per-thread attribute. Even if we could figure out a way to change that--nothing comes to mind--we could not change it now because it would break existing Go programs that call os.Chdir in one goroutine and expect it to affect another goroutine.

Closing as unfortunate.

@blissnd
Copy link
Author

blissnd commented Sep 13, 2018 via email

@ianlancetaylor
Copy link
Contributor

Does Go support something equivalent to:

https://docs.python.org/3.4/library/multiprocessing.html?highlight=process

?

So that it could be done by forking a new process instead of spawning a new thread?

If I understand the Python docs correctly, the answer is no. It's basically impossible to implement that kind of API in a multi-threaded program on Unix systems. Of course you can start a new process using the os/exec package, and you could use os.Executable to start your own program in a new process, but you would have to introduce some mechanism to tell the new process what to do.

We don't use the issue tracker for discussion, so you will get better answers for any followup questions on the golang-nuts mailing list or some other forum; see https://golang.org/wiki/Questions.

@blissnd
Copy link
Author

blissnd commented Sep 15, 2018

What is especially shocking is that Java copes with this use case perfectly:

JAVA CASE

Java copes perfectly well with the same use case. To run the java:

cd JAVA_TEST

javac *.java

java Concurrency_Test

It works perfectly every time with a very similar multi-threaded use case with Java (OpenJDK 1.8). So my question is why doesn't GoLang behave similary (i.e.reasonably) like Java does...???

EVIDENCE:

https://github.com/blissnd/BUG_IN_GOLANG/tree/master/JAVA_TEST

@ianlancetaylor
Copy link
Contributor

As far as I can tell your Java program does not call the equivalent of os.Chdir. It does the equivalent of setting the Dir field in os/exec.Cmd. That works in Go also.

@as
Copy link
Contributor

as commented Sep 16, 2018

The current working directory information is stored in a process-scoped environment block. This is a true statement for all operating systems supported by Go that I know of (except for Plan 9). Windows NT does not support fork(), and even has incompatibilities passing file descriptors to new processes in the current api. Furthermore, how would those two processes communicate across different copies of the runtime even if fork could work across all systems?

Edit: Plan 9 does store working directory per thread

@blissnd
Copy link
Author

blissnd commented Sep 16, 2018

Ok, but is runtime.LockOSThread behaving as expected? The way it's documented, I would expect it to lock the OS thread in order to make os.chdir() thread-safe:

https://github.com/blissnd/BUG_IN_GOLANG/blob/workaround/test_concurrency.go

@as
Copy link
Contributor

as commented Sep 17, 2018

I would expect it to lock the OS thread

This would only apply on systems like Plan 9 (which store a working directory per thread, see previous correction). The other systems store it in a process environment block. Locking the goroutine to one thread won't change the results--the working directory will change for the entire process.

@tv42
Copy link

tv42 commented Feb 12, 2019

For posterity: the native-Linux way to do this is openat(2) and friends. Not tied to any threads or anything, any FD can be used as a directory reference. Go does not expose a high-level interface to such, at this time.

@golang golang locked and limited conversation to collaborators Feb 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants