[ruby-core:91807] [Ruby trunk Bug#15661] Disallow concurrent Dir.chdir with block

[ruby-core:91807] [Ruby trunk Bug#15661] Disallow concurrent Dir.chdir with block

[headius]

Issue #15661 has been reported by headius (Charles Nutter).

----------------------------------------
Bug #15661: Disallow concurrent Dir.chdir with block
https://bugs.ruby-lang.org/issues/15661

* Author: headius (Charles Nutter)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: all
* Backport: 2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
Dir.chdir with a block should disallow concurrent use, since it will almost never produce the results a user expects.

In https://bugs.ruby-lang.org/issues/9785 calls for Dir.chdir to be made thread-safe were rejected because the underlying native call is process-global. This is reasonable because there's no way to easily make the native chdir be thread-local (at least not without larger changes to CRuby itself).

However the block form of chdir is explicitly bounded, and clearly indicates that the dir should be changed only for the given block. I believe Dir.chdir should prevent multiple threads from attempting to do this bounded chdir at the same time.

Currently, if two threads attempt to do a Dir.chdir with a block, one of them will see a warning: "conflicting chdir during another chdir block". This warning is presumably intended to inform the user that they may see unpredictable results, but I can think of no cases where you would ever see predictable results.

In other words, there's no reason to allow a user to do concurrent Dir.chdir with a block because they will always be at risk of unpredictable results, and I believe this only leads to hard-to-diagnose bugs.

The warning should be a hard error.

The warning should also be turned into an error if a non-block Dir.chdir call happens while a block Dir.chdir is in operation. The important thing is to protect the integrity of the current directory during the lifetime of that block invocation.

In CRuby terms, the patch would be to check for chdir_blocking > 0 and then simply error out, unless it's happening on the same thread.

Concurrent non-block Dir.chdir would be unaffected. This only protects the block form from having the current dir change while it is executing.

See https://github.com/jruby/jruby/issues/5649



-- 
https://bugs.ruby-lang.org/

[ruby-core:91811] [Ruby trunk Bug#15661] Disallow concurrent Dir.chdir with block

[shevegen]

Issue #15661 has been updated by shevegen (Robert A. Heiler).


I sort of agree with headius. I also can not really think of a possible and
deliberate use case for concurrent Dir.chdir with a block. Perhaps something
is missing (someone has a use case?) but otherwise I sort of agree with him
here.

----------------------------------------
Bug #15661: Disallow concurrent Dir.chdir with block
https://bugs.ruby-lang.org/issues/15661#change-77085

* Author: headius (Charles Nutter)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: all
* Backport: 2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
Dir.chdir with a block should disallow concurrent use, since it will almost never produce the results a user expects.

In https://bugs.ruby-lang.org/issues/9785 calls for Dir.chdir to be made thread-safe were rejected because the underlying native call is process-global. This is reasonable because there's no way to easily make the native chdir be thread-local (at least not without larger changes to CRuby itself).

However the block form of chdir is explicitly bounded, and clearly indicates that the dir should be changed only for the given block. I believe Dir.chdir should prevent multiple threads from attempting to do this bounded chdir at the same time.

Currently, if two threads attempt to do a Dir.chdir with a block, one of them will see a warning: "conflicting chdir during another chdir block". This warning is presumably intended to inform the user that they may see unpredictable results, but I can think of no cases where you would ever see predictable results.

In other words, there's no reason to allow a user to do concurrent Dir.chdir with a block because they will always be at risk of unpredictable results, and I believe this only leads to hard-to-diagnose bugs.

The warning should be a hard error.

The warning should also be turned into an error if a non-block Dir.chdir call happens while a block Dir.chdir is in operation. The important thing is to protect the integrity of the current directory during the lifetime of that block invocation.

In CRuby terms, the patch would be to check for chdir_blocking > 0 and then simply error out, unless it's happening on the same thread.

Concurrent non-block Dir.chdir would be unaffected. This only protects the block form from having the current dir change while it is executing.

See https://github.com/jruby/jruby/issues/5649



-- 
https://bugs.ruby-lang.org/

[ruby-core:91826] [Ruby trunk Bug#15661] Disallow concurrent Dir.chdir with block

[nobu]

Issue #15661 has been updated by nobu (Nobuyoshi Nakada).

Description updated

Why not blocking until another block terminates?

----------------------------------------
Bug #15661: Disallow concurrent Dir.chdir with block
https://bugs.ruby-lang.org/issues/15661#change-77096

* Author: headius (Charles Nutter)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: all
* Backport: 2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
`Dir.chdir` with a block should disallow concurrent use, since it will almost never produce the results a user expects.

In https://bugs.ruby-lang.org/issues/9785 calls for `Dir.chdir` to be made thread-safe were rejected because the underlying native call is process-global. This is reasonable because there's no way to easily make the native chdir be thread-local (at least not without larger changes to CRuby itself).

However the block form of `chdir` is explicitly bounded, and clearly indicates that the dir should be changed only for the given block. I believe `Dir.chdir` should prevent multiple threads from attempting to do this bounded `chdir` at the same time.

Currently, if two threads attempt to do a `Dir.chdir` with a block, one of them will see a warning: "conflicting chdir during another chdir block". This warning is presumably intended to inform the user that they may see unpredictable results, but I can think of no cases where you would ever see predictable results.

In other words, there's no reason to allow a user to do concurrent `Dir.chdir` with a block because they will always be at risk of unpredictable results, and I believe this only leads to hard-to-diagnose bugs.

The warning should be a hard error.

The warning should also be turned into an error if a non-block `Dir.chdir` call happens while a block Dir.chdir is in operation. The important thing is to protect the integrity of the current directory during the lifetime of that block invocation.

In CRuby terms, the patch would be to check for `chdir_blocking > 0` and then simply error out, unless it's happening on the same thread.

Concurrent non-block `Dir.chdir` would be unaffected. This only protects the block form from having the current dir change while it is executing.

See https://github.com/jruby/jruby/issues/5649



-- 
https://bugs.ruby-lang.org/

[ruby-core:91839] [Ruby trunk Bug#15661] Disallow concurrent Dir.chdir with block

[headius]

Issue #15661 has been updated by headius (Charles Nutter).


@nobu I actually considered that my first preference, except for the introduction of a new global lock.

I worry about suddenly introducing deadlocks into someone's code, though that code would have to be doing some locking as well as a concurrent chdir somewhere (both locks need to be contended). Perhaps that code deserves what it got, but then again maybe it's better to just have a hard error anyway. I'm willing to discuss it more. 

----------------------------------------
Bug #15661: Disallow concurrent Dir.chdir with block
https://bugs.ruby-lang.org/issues/15661#change-77108

* Author: headius (Charles Nutter)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: all
* Backport: 2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
`Dir.chdir` with a block should disallow concurrent use, since it will almost never produce the results a user expects.

In https://bugs.ruby-lang.org/issues/9785 calls for `Dir.chdir` to be made thread-safe were rejected because the underlying native call is process-global. This is reasonable because there's no way to easily make the native chdir be thread-local (at least not without larger changes to CRuby itself).

However the block form of `chdir` is explicitly bounded, and clearly indicates that the dir should be changed only for the given block. I believe `Dir.chdir` should prevent multiple threads from attempting to do this bounded `chdir` at the same time.

Currently, if two threads attempt to do a `Dir.chdir` with a block, one of them will see a warning: "conflicting chdir during another chdir block". This warning is presumably intended to inform the user that they may see unpredictable results, but I can think of no cases where you would ever see predictable results.

In other words, there's no reason to allow a user to do concurrent `Dir.chdir` with a block because they will always be at risk of unpredictable results, and I believe this only leads to hard-to-diagnose bugs.

The warning should be a hard error.

The warning should also be turned into an error if a non-block `Dir.chdir` call happens while a block Dir.chdir is in operation. The important thing is to protect the integrity of the current directory during the lifetime of that block invocation.

In CRuby terms, the patch would be to check for `chdir_blocking > 0` and then simply error out, unless it's happening on the same thread.

Concurrent non-block `Dir.chdir` would be unaffected. This only protects the block form from having the current dir change while it is executing.

See https://github.com/jruby/jruby/issues/5649



-- 
https://bugs.ruby-lang.org/