add a mutex to atomic_write

[ahorek]

refs https://github.com/rails/sprockets/issues/136

[hurricup]

Btw, feels like it does not close #136 actually. Symptoms and reasons are the same, but in other place. Original report was about touch but i have never seen this myself, so can't say for sure.

Thanks for PR, my ruby is lame to make it right at once.

[ahorek]

I've added a simple test that reveals adding a Mutex doesn't help. The file is probably being opened somewhere else.

I'm no longer using sprockets, but if someone wants to dig deeper, feel free to do so :)

[hurricup]

Can this be some other failure? Because mutex fixes my scenario for 100% in my case it was a failure for rails assets:precompile

[ahorek]

It doesn't happen if the file is cached already. Did you remove the cache?

even tho concurrency issues are a matter of luck, as you can see the first build passes and the second one failed https://ci.appveyor.com/project/rafaelfranca/sprockets/builds/45745367/job/l9a553if6tyyfagl https://ci.appveyor.com/project/rafaelfranca/sprockets/builds/45745367/job/lgd52t8o1ipqolwg

[hurricup]

Of course. Remove the cache, run rake task. It fails first time. I don't really understand what test does, so can't say :) Oh, well, I see. yes, probably images opened somewhere else. This fixes caches, not like everything :)

[ahorek]

ok, it should pass now. @hurricup it would be great if you could test the change on a real app. Thanks

[hurricup]

@ahorek actually I don't have one :) I'm working on RubyMine and we have integration tests. And some of them are flickering on windows agents because of this :)

And our particular issue was fixed by mutexing rename. Still, feels like would be nice to have the way to disable muti-threading for some/all stuff here.

[ahorek]

I tested it on an old app and there are still concurrency issues. I don't have plans to work on it further, so I'll close the PR.

as a workaround disable concurrent exports:

Rails.application.configure do
  config.assets.configure do |env|
    env.export_concurrent = false
  end
end

[MaxLap]

I had the problem during assets:precompile.

Errno::EACCES: Permission denied @ rb_file_s_rename - (E:/my_app/tmp/cache/assets/sprockets/v4.0.0/eV/eV7fIEa3XVHb6TIgPAxpDOAJkfKj-RJmawTi0JplsRk.cache.23580.23892.464759, E:/my_app/tmp/cache/assets/s
prockets/v4.0.0/eV/eV7fIEa3XVHb6TIgPAxpDOAJkfKj-RJmawTi0JplsRk.cache)
C:/Ruby27-x64/lib/ruby/gems/2.7.0/gems/sprockets-4.2.0/lib/sprockets/path_utils.rb:362:in `rename'
C:/Ruby27-x64/lib/ruby/gems/2.7.0/gems/sprockets-4.2.0/lib/sprockets/path_utils.rb:362:in `atomic_write'
C:/Ruby27-x64/lib/ruby/gems/2.7.0/gems/sprockets-4.2.0/lib/sprockets/cache/file_store.rb:112:in `set'
C:/Ruby27-x64/lib/ruby/gems/2.7.0/gems/sprockets-4.2.0/lib/sprockets/cache.rb:227:in `set'
...

I can confirm that adding this to my application.rb fixes the issue (I deleted the tmp/assets folder each time):

config.assets.configure do |env|
  env.export_concurrent = false
end

I tested (without the config) using the part of this PR that touches the "rename" (I didn't bother with the safe_open). I just edited the sprockets code in the gem folder. It also solvedsthe problem on my side.

So in my book, this should definitely be merged to help with at least some problems. It's a few hours that I would not have had to waste here.

Thanks ahorek