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

Asset compilation stalls with sprockets v4 #640

Open
leoarnold opened this issue Oct 16, 2019 · 28 comments · May be fixed by #801
Open

Asset compilation stalls with sprockets v4 #640

leoarnold opened this issue Oct 16, 2019 · 28 comments · May be fixed by #801

Comments

@leoarnold
Copy link

Expected behavior

Asset compilation speed with sprockets v4.0.0 should be comparable to or faster than with v3.7.2.

Actual behavior

When calling rails assets:clobber tmp:clear assets:precompile sprockets will start building the cache (i.e. tmp/cache/assets/sprockets/v4.0.0/) up to about 8 MB (full cache with v3.7.2: 18MB) with no assets written and then the process and all of its six forks seem to come to a complete halt: 0.0% CPU usage according to htop. The process will not react to SIGINT or SIGTERM any more, only SIGHUP and SIGKILL can end the process.

Same with assets compilation in Rails development environment upon first request.

Once sprockets cache is built, sprockets will run smooth and quick as expected, but building the cache seemingly takes forever: Travis CI will just abort the build after 10 minutes.

System configuration

  • Sprockets version 4.0.0
  • Ruby version 2.5.0
  • Rails version 6.0.0
  • execjs 2.7.0
  • coffee-rails 5.0.0 / coffee-script 2.4.1
  • uglifier 4.2.0
  • sass 3.4.25 / sassc-rails 2.1.4 / sassc 2.2.1

Example App (Reproduction)

I couldn't quite figure out what is making sprockets stall so I am not able to provide an example app at time of this writing (project in question is private company property).

Therefore I would be very grateful if the community could provide pointers on how I could debug this and identify the root cause.

@leoarnold
Copy link
Author

Using git bisect I identified this as the first "bad" commit: a3e3a40

When I branch off sprockets v4.0.0 and revert this commit asset compilation works just fine again. Hence I think the root cause of my problem lies in that commit.

@jeremy
Copy link
Member

jeremy commented Oct 16, 2019

References #469

@gingerlime
Copy link

also bumped into this within docker... Oddly on one machine it seems to work with the exact same build / code, but another one (with more CPUs maybe slightly different architecture, both are on Digital Ocean) it hangs the asset precompile exactly the same way that @leoarnold described... Would be tricky to build a reproducible example unfortunately, but happy to run some tests etc.

We just upgraded sprockets to 4.0.0 and with 3.7.2 it didn't seem to happen.

@leoarnold
Copy link
Author

leoarnold commented Oct 26, 2019

As far as I understand #469 tried to pivot from asset compilation path after path to all paths in parallel. Therefore we need to wait for all compilers to finish before moving on:

promises.each(&:wait!)

This means we are waiting for all promises to complete with nil timeout

https://github.com/ruby-concurrency/concurrent-ruby/blob/ffed3c3c0518030b0ed245637703089fa1f0eeee/lib/concurrent/concern/obligation.rb#L79-L89

which can cause indefinite waiting as reported above. Waiting is a synchronized method

https://github.com/ruby-concurrency/concurrent-ruby/blob/ffed3c3c0518030b0ed245637703089fa1f0eeee/lib/concurrent/atomic/event.rb#L78-L92

where the underlying implementation of the locking mechanism varies with the Ruby interpreter used (but for most use cases it presumably is a mutex):

https://github.com/ruby-concurrency/concurrent-ruby/blob/ffed3c3c0518030b0ed245637703089fa1f0eeee/lib/concurrent/synchronization/lockable_object.rb#L6-L20

It still remains to investigate what can cause a promise to never finish (i.e. its event to never become "set").

Maybe the choice of the :executor can yield some insight:

def executor
@executor ||= environment.export_concurrent ? :fast : :immediate
end

@gingerlime Would you mind running some builds with Sprockets.export_concurrent = false (defaults to true)? Does this also end up in a deadlock in some runs?

@gingerlime
Copy link

@leoarnold sorry, but where exactly do I set Sprockets.export_concurrent = false in?

@gingerlime
Copy link

related? #534

@gingerlime
Copy link

attached 2 straces in a zip file... one was stalling, and the other one was eventually successful (for the exact same codebase/command)

strace.zip

@leoarnold
Copy link
Author

@gingerlime I set Sprockets.export_concurrent = false in a Rails initializer in our project and asset compilation worked as expected, so it all seems to be about the use of the :fast executer.

@leoarnold
Copy link
Author

leoarnold commented Oct 28, 2019

@gingerlime I think I'm onto it: I think Sprockets is deadlocking because it tries to compile some assets twice in parallel. I think I missed the last paragraph of

https://eileencodes.com/posts/the-sprockets-4-manifest/

where it tells you to remove the line with config.assets.precompile from your config/initializers/assets.rb.

I just yanked that line and now Travis CI runs as expected.

@gingerlime
Copy link

gingerlime commented Oct 28, 2019

@leoarnold we removed the config.assets.precompile from our initializers and moved things over to app/assets/config/manifest.js but we're still seeing these issues... :-/

Here's what the PR looked like (after merging this, we started seeing those issues)

Screen Shot 2019-10-28 at 19 52 21

@leoarnold
Copy link
Author

@gingerlime To get my manifest right, I temporarily added the line

pp args.flatten.map { |path| [path, environment.find_all_linked_assets(path).map(&:filename)] }.to_h

in between these two

environment = self.environment.cached
promises = args.flatten.map do |path|

and run rails assets:precompile.

As far as I understand, the output should look like a hash with only manifest.js as key and none of the assets mentioned twice.

@gingerlime
Copy link

gingerlime commented Oct 30, 2019

Thanks for the tip, @leoarnold.

I see other keys besides manifest.js (perhaps being added by other gems like CKEditor, ActiveAdmin etc?)...

For the manifest.js key there are a few duplicates (a few images and fonts), but I'm really unsure why they are generated.

In any case, wouldn't it be more sensible for sprockets to just walk over all unique entries if duplicates are causing deadlocks?

@leoarnold
Copy link
Author

@gingerlime Out of curiosity: Why do you only link the directory javascripts/ckeditor instead of all of javascripts?

@gingerlime
Copy link

@leoarnold probably better if my colleague @joker-777 comments to confirm this, but as far as I'm aware, our main app javascript is compiled using webpack(er), but ActiveAdmin, CKEditor and our CSS and a few other bits don't support it (yet?) so they need to use the standard asset precompile process.

@leoarnold
Copy link
Author

@gingerlime Can you post an (obfuscated) example of your asset list with the duplicates. Maybe we can construct a failing unit test from it in order to draw some core contributers' interest to this issue.

@gingerlime
Copy link

@leoarnold I was hoping that the strace files I shared would help pinpoint the issue? but not sure if anyone had looked at those?

Anyway, here's the manifest output from adding the line as you suggested. I don't think there's anything particularly sensitive about it. All those assets are public eventually :)

manifest.txt

@nhoizey
Copy link

nhoizey commented Nov 7, 2019

It looks like I have this stalled compilation when trying to use Sprockets 4 with Jekyll Assets 4 in Jekyll 4…

envygeeks/jekyll-assets#613 (comment)

@leoarnold
Copy link
Author

@gingerlime AFAIK the strace can only show what is happening, but not why. In the current understanding of the problem (as per this thread right here), I think we are clear that

  • we are running into a deadlock
  • two concurrent phenomena are trying to access the same file
  • it did not occur before, because files were processed successively before (the equivalent of Sprockets.export_concurrent = false now)
    The question now is how to prevent Sprockets from deadlocking itself.

In your debug output I noticed that any files required ckeditor/contents.css is also requested by manifest.js three times.

It seems that you are using stylesheet_link_tag "ckeditor/contents.css" (or the HTML equivalent) in your codebase. Try adding //= link ../ckeditor/contents.css to you manifest - especially since manifest.js already requires /app/app/assets/stylesheets/ckeditor/contents.css. Same goes for anything else you import with stylesheet_link_tag, javascript_include_tag or their HTML equivalents.

Does that change anything for you?

My working assumption is that we need a uniq here

environment.find_all_linked_assets(path) do |asset|

or an improvement to find_all_linked_assets, and a check that any two members of the array

args.flatten.map { |path| environment.find_all_linked_assets(path).map(&:filename) }

must be disjoint.

@gingerlime
Copy link

Thanks @leoarnold we'll look into it (@joker-777?)

Definitely feels like adding a uniq makes sense. Even if there would be no deadlocks, what's the point of precompiling assets more than once? :)

@joker-777
Copy link

@leoarnold Thanks a lot for your help and suggestions. //= link ../ckeditor/contents.css didn't work because the path is wrong but //= link ckeditor/contents.css which I now added instead of //= link_directory ../stylesheets/ckeditor .css, which was there before. Unfortunately it didn't change anything in the manifest output.

Please let me know if maybe misunderstood something.

You also asked, why we add javascripts/ckeditor separately. I'm not 100% sure anymore why but at least from the ckeditor documentation I can see that you have to recompile ckeditor/config.js separately.

/cc @gingerlime

@shepmaster
Copy link

I've experienced the same problem. Upgrading Rails from 5 to 6 also upgraded sprockets at the same time, which complicated narrowing down the problem!

Using Sprockets.export_concurrent = false did appear to remove the deadlock for me.

@glaszig
Copy link

glaszig commented Dec 23, 2019

slightly different on my side: rails assets:precompile works. but on-the-fly compilation from a running rails app in development stalls the entire process.

Sprockets.export_concurrent = false fixes it for me.

@goulvench
Copy link

Sprockets.export_concurrent = false doesn't fix asset:precompile for me.

I have the following setup (on a Rails 6 app with both webpack and Sprockets assets), and the deadlock happens (right after yarn:install) even if I slim down to 1 scss and 1 js script:

// vendor.scss
@import 'bootstrap';
@import 'font-awesome';

// application.scss
@import 'bootstrap/functions';
@import 'bootstrap/variables';
@import 'bootstrap/mixins';
@import 'partials/yellow_fade';
@import 'partials/transformicon';

Is there a way to monkey-patch around this for the time being? Thanks.

@fxtentacle
Copy link

Same issue here. I tried unicorn, puma, thin, but each one would just deadlock with 0% CPU on the first access in development mode. Adding

if Rails.env.development?
  Sprockets.export_concurrent = false
end

to application.rb fixed the problem.

@wjordan
Copy link
Contributor

wjordan commented May 8, 2020

See also #538 which seems to be an earlier report of the same issue.

I managed to narrow down a hang/deadlock in concurrent-ruby which seems to be a bug in the Concurrent::Promise implementation, and filed an issue upstream: ruby-concurrency/concurrent-ruby#870. [edit: This #zip bug may only affect the 3.x backport (#470) I have been using, but there may be other related bugs in Concurrent::Promise triggering this issue for others.]

In the meantime, it seems that concurrent-ruby has rewritten their promises implementation in v1.1 (released Nov 2018) to be more stable/performant but under a separate Concurrent::Promises API, so modifying Promise to Promises in this code might eliminate (at least some of) the unstable hanging/deadlock issues appearing in concurrent asset compilation.

@tomkr4l
Copy link

tomkr4l commented Sep 15, 2020

I was able to resolve this issue by regressing back to version 3.7.2 which I've used before upgrading to Rails 6. Here is log from stacktrace, maybe It will help to resolve issue.

stacktrace.txt
error

@ryanb
Copy link

ryanb commented Sep 21, 2021

I recently ran into rails assets:precompile hanging and found out it was due to a misconfigured assets_sync. Posting this here in case someone else runs into the same issue.

@aharpervc
Copy link

@leoarnold

My working assumption is that we need a uniq here

environment.find_all_linked_assets(path) do |asset|

or an improvement to find_all_linked_assets, and a check that any two members of the array

args.flatten.map { |path| environment.find_all_linked_assets(path).map(&:filename) }

must be disjoint.

I can confirm that adding uniq as suggested takes sprockets from basically hung to working perfectly! This was driving me NUTS, thank you!!

@olivier-thatch olivier-thatch linked a pull request Feb 2, 2024 that will close this issue
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 a pull request may close this issue.