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

Commas in directory names causes hang in ActiveSupport::FileUpdateChecker #5448

Merged
merged 1 commit into from
Mar 15, 2012

Conversation

JonRowe
Copy link
Contributor

@JonRowe JonRowe commented Mar 15, 2012

Having commas in directory names passed through to ActiveSupport::FileUpdateChecker causes an infinite hang whilst Dir[@glob] tries and fails to list files in these directories. This wasn't an issue before ~> 3.2 but on 3.2 upwards it prevents Rails from booting in development mode and prevents test suites running (rspec, cucumber at least).

Commas are perfectly valid parts of directory names (on OS X at least) and escaping them when compiling the glob solves this issue.

This issue can be triggered by adding such directories to the autoload component of rails config see: https://github.com/JonRowe/rails-broken-dir-example for an example broken application.

This is a duplicate of #5445 but, as requested, against master and without fork.

Be warned on OSX/MRI Ruby this causes an infinite hang on test failure...

@josevalim
Copy link
Contributor

Be warned on OSX/MRI Ruby this causes an infinite hang on test failure...

Thanks. But I am a bit confused. This patch will cause an infinite hang even with the bug fixed? o.O

@JonRowe
Copy link
Contributor Author

JonRowe commented Mar 15, 2012

No only if the test fails

@josevalim
Copy link
Contributor

Which hopefully will rarely happen, confirm/deny? Also, shouldn't the timeout catch the hanging?

@JonRowe
Copy link
Contributor Author

JonRowe commented Mar 15, 2012

Shouldn't happen as long as comma's are escaped. So confirm.

Timeout seems to use thread, but neither thread nor timeout catch the hanging on MRI ruby. It seems Dir.glob goes straight into C and blocks somewhere low down in Ruby, beyond the reach of the thread scheduler / GIL

My theory is that without escaping you are essentially globing the entirety of the filesystem, if you leave an unpatched version going long enough on OS X you get access denied errors for various private system directories bubbling up.

@josevalim
Copy link
Contributor

Awesome, thanks for the explanation and the fixes.

josevalim added a commit that referenced this pull request Mar 15, 2012
Commas in directory names causes hang in ActiveSupport::FileUpdateChecker
@josevalim josevalim merged commit ee8e567 into rails:master Mar 15, 2012
@JonRowe
Copy link
Contributor Author

JonRowe commented Mar 15, 2012

Thanks :)

@JonRowe
Copy link
Contributor Author

JonRowe commented Mar 15, 2012

For the benefit of anybody else running into this, it can also be triggered purely by having directories with comma's in the name in your app folder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants