berkshelf / ridley

A reliable Chef API client with a clean syntax
Other
231 stars 85 forks source link

Replaced file locking code #281

Closed diadistis closed 9 years ago

diadistis commented 9 years ago

This fixes the following error :

`delete': Permission denied - [some path]/metadata.json (Errno::EACCES)

Details about the problem can be found in this SO question

sethvargo commented 9 years ago

@diadistis thank you for the PR and I'm sorry this is causing you problems. I'm actually :-1: on this as I think switching from {} to do..end isn't actually solving your problem. If it is, then that is a serious Ruby bug because those closures should behave exactly the same. If the different behavior is intended, then there is definitely a documentation bug. @skottler have you seen anything like this before?

From the pastebin, I can determine you are running ChefDK with embedded Ruby 2.0.0. I couldn't find an upstream bug about this on bugs.ruby-lang.org, but that doesn't mean it doesn't exist :smile:. I haven't been keeping up with ChefDK releases, but perhaps you could try upgrading to the latest version? An upgraded Ruby version may fix this issue. @danielsdeleo @sersut has anyone else reported an issue like this? It's definitely a weird one to me...

diadistis commented 9 years ago

@sethvargo thanks for the feedback. Yes there are others who seem to have the same problem. It maybe a version/platform combination. I will look further into it.

sethvargo commented 9 years ago

@diadistis well I've upvoted and tweeted your Stackoverflow post because I'm also curious. The {} and do..end syntaxes are not 100% equal ({} is a "stronger" closure than do..end), but I do not think that should make a difference here...

diadistis commented 9 years ago

@sethvargo thanks. I have tested it again and again. It does make a difference in my setup. I will try to recreate the problem with a simpler ruby script.

danielsdeleo commented 9 years ago

I haven't seen any other reports like this on the ChefDK issues, Chef mailing list, etc., sorry. This is very bizarre, as the only documented difference between the two block syntaxes is the precedence (as @sethvargo says), which should only make a difference if you are using so-called "Seattle Style" ruby (never use parens unless absolutely required).

For a wild guess, if this code is run in threads, perhaps one style allows ruby to context switch to a new thread and the other doesn't? Could this be a very bizarre symptom of an underlying race condition?

skottler commented 9 years ago

This is almost certainly a race condition in the language's implementation. Pretty such a shot in the dark (and this needs more investigation), but the issue seems to arise here:

vm_get_ruby_level_caller_cfp(th, RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp));

The curly-brace vs. do block syntaxes seem to return different cfp's in the line above (from vm.c in core). Precedence should not matter here.

MattVonVielen commented 9 years ago

I think it's important to note that everybody encountering this issue (Including myself) is running ChefDK on Windows, so this feels like it may be a platform-specific issue in addition to what @skottler found above.

diadistis commented 9 years ago

@xenolinguist can you please check if my patch works for you too?

carpnick commented 9 years ago

+1 - seeing it intermittently on Windows 7 here. Same stack trace.

reset commented 9 years ago

I'm going to go ahead and accept this no matter how bizzare it sounds.