Loading Incorrect Images #57

Closed
sorentwo opened this Issue Apr 5, 2012 · 12 comments

Comments

Projects
None yet
6 participants
Contributor

sorentwo commented Apr 5, 2012

Images (notably css background images) are entirely incorrect, showing a different image. This hasn't surfaced in Webrick or Unicorn. I've seen it a few times in development, but it happens consistently in staging. Taking a look in the inspector and in logs the urls appear to be correct, but there is no mistaking that the image itself is mixed up—the logo being replaced with a background tile, for example.

Verified in Chrome, Safari, and Firefox (hard reload in each of them). The interchanged images aren't the same across browsers, but the effect is present in all of them.

Here is the basic stack, ff there is any additional info I can provide let me know.

Ruby 1.9.3-p125, Rails 3.2.2, Puma with default of 0-16 threads.

Owner

evanphx commented Apr 10, 2012

Do you have an reproduction you could show me? Perhaps a rails app?

Contributor

sorentwo commented Apr 13, 2012

The app I was testing this out with is our production app, and I can't share it exactly. I can share the gemspec and environment files though.

https://gist.github.com/2375692

Only the development config is included, but this would happen in all environments.

Owner

evanphx commented Apr 13, 2012

Any chance you could build a sample rails app that shows the problem? It's quite difficult to derive the situation from just your config files.

ezkl commented Apr 13, 2012

@sorentwo Do you still see the issue if you turn off the asset pipeline by setting config.assets.enabled = false in config/application, run RAILS_ENV=development bundle exec rake assets:precompile and then run the server?

Contributor

sorentwo commented Apr 17, 2012

Truthfully I returned to using Unicorn after running into the issue, so it took me some time to get a free moment to investigate this.

@ezkl Those precise steps don't quite work out, but not because of anything with Puma. With the asset pipeline disabled no precompilation happens at all, and with it enabled no stylesheets are precompiled, making the exercise futile.

@evanphx I'll try to do this soon. Because the image mixup is random on different servers, and we're loading 20+ images on the affected pages I fear it would be difficult to replicate.

I've seen the same issue myself. I found Rack Cache seemed to be misbehaving, although I didn't get to diagnose the specific cause.

@sorentwo Are you using Rack Cache? I do know Rails 3 injects it into the middleware stack by default.

Contributor

sorentwo commented Apr 17, 2012

Yeah, the app is using Rack::Cache. In staging and production it is configured with Memcached/Dalli, nothing is configured in development.

jtblin commented Aug 4, 2012

I can repro with Rack::Cache. It is quite simple to repro, just add rack-cache gem, sets config.consider_all_requests_local = true, config.serve_static_assets = true and config.static_cache_control = "public, max-age=2592000" development.rb in a project where you have css, images, etc. Just refresh the pages a few time.

It seems to occur because puma sets env['rack.run_once'] to true and Rack::Cache determines if it's going to make a thread safe request based on this, e.g. /lib/rack/cache/context.rb:48
def call(env)
if env['rack.run_once']
call! env
else
clone.call! env
end
end

Either puma should not set env['rack.run_once'] to true or Rack::Cache should test if env['rack.multithread'] is not true as well, i.e.

if env['rack.run_once'] && !env['rack.multithread']
call! env
else
clone.call! env
end

I'll propose this fix to rack::cache project and let this thread know if they accept my pull request.

rtomayko commented Aug 5, 2012

I just merged @jtblin change in rtomayko/rack-cache#71 so that rack.multithread is checked but puma setting rack.run_once seems pretty wrong here. What's the reasoning behind setting it true in this case?

Owner

evanphx commented Aug 5, 2012

Puma shouldn't be setting rack.run_once, that's my mistake. I'm finishing up testing of 1.7.0 today and will have it out perhaps shortly and it will include removing setting rack.run_once. I'll also review all the rack options that are set.

@evanphx evanphx closed this in 4c2f6b4 Aug 5, 2012

Contributor

sorentwo commented Aug 6, 2012

Thanks @jtblin, @rtomayko and @evanphx. I never got back to trying to work through this and it haunted me a bit. Awesome to see that this got resolved though.

jtblin commented Aug 6, 2012

Thanks a ton @rtomayko and @evanphx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment