ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
From: daniel@dan42.com
To: ruby-core@ruby-lang.org
Subject: [ruby-core:100503] [Ruby master Bug#15661] Disallow concurrent Dir.chdir with block
Date: Thu, 22 Oct 2020 14:15:05 +0000 (UTC)	[thread overview]
Message-ID: <redmine.journal-88123.20201022141504.286@ruby-lang.org> (raw)
In-Reply-To: redmine.issue-15661.20190313121953.286@ruby-lang.org

Issue #15661 has been updated by Dan0042 (Daniel DeLorme).


From the bug report, I got the idea this was about preventing a chdir while a chdir with block was active _in another thread_. That sounds fine to me, but this code also triggers a warning:

```ruby
Dir.chdir("/usr") do
  #do things in /usr
  Dir.chdir("local") #warning: conflicting chdir during another chdir block
  #do things in /usr/local until the end of the block
end
```

If we're talking about turning this as well into an error I'm not sure I agree.

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

* Author: headius (Charles Nutter)
* Status: Closed
* Priority: Normal
* 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/

  parent reply	other threads:[~2020-10-22 14:15 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <redmine.issue-15661.20190313121953.286@ruby-lang.org>
2020-09-25  5:30 ` [ruby-core:100122] [Ruby master Bug#15661] Disallow concurrent Dir.chdir with block matz
2020-09-25 20:36 ` [ruby-core:100161] " merch-redmine
2020-09-26 11:27 ` [ruby-core:100173] " sam.saffron
2020-09-26 21:20 ` [ruby-core:100182] " merch-redmine
2020-09-26 21:29   ` [ruby-core:100183] " Eric Wong
2020-09-27 10:01 ` [ruby-core:100186] " eregontp
2020-09-28  0:07 ` [ruby-core:100190] " sam.saffron
2020-09-28  9:39 ` [ruby-core:100198] " eregontp
2020-10-01  7:00 ` [ruby-core:100247] " jean.boussier
2020-10-01  7:03 ` [ruby-core:100248] " jean.boussier
2020-10-17 17:43 ` [ruby-core:100420] " jonathan
2020-10-22 14:15 ` daniel [this message]
2020-12-08  3:50 ` [ruby-core:101302] " nobu
2020-12-24  4:50 ` [ruby-core:101671] " mame

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.ruby-lang.org/en/community/mailing-lists/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=redmine.journal-88123.20201022141504.286@ruby-lang.org \
    --to=ruby-core@ruby-lang.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).