Asset compilation stalls with sprockets v4

[leoarnold]

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]

Using git bisect I identified this as the first "bad" commit: https://github.com/rails/sprockets/commit/a3e3a40bb509586e0e325f5057a1637ec8da0cb9

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]

References #469

[gingerlime]

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]

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:

https://github.com/rails/sprockets/blob/08fef08562c7a6a13a7c521938e83409a33e2b77/lib/sprockets/manifest.rb#L130

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:

https://github.com/rails/sprockets/blob/08fef08562c7a6a13a7c521938e83409a33e2b77/lib/sprockets/manifest.rb#L334-L336

@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]

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

[gingerlime]

related? #534

[gingerlime]

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]

@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]

@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]

@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)

[leoarnold]

@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

https://github.com/rails/sprockets/blob/c4b191e70d89e9d70f19ade5faf0692054a3bd1b/lib/sprockets/manifest.rb#L122-L123

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]

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]

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

[gingerlime]

@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]

@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]

@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]

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

https://github.com/envygeeks/jekyll-assets/issues/613#issuecomment-551142766

[leoarnold]

@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

https://github.com/rails/sprockets/blob/08fef08562c7a6a13a7c521938e83409a33e2b77/lib/sprockets/manifest.rb#L125

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]

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]

@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]

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]

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]

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]

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]

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]

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

[ryanb]

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]

@leoarnold

My working assumption is that we need a uniq here

https://github.com/rails/sprockets/blob/08fef08562c7a6a13a7c521938e83409a33e2b77/lib/sprockets/manifest.rb#L125

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!!