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

Fixes #19229: Deadlock at start when loading new licence #380

Conversation

fanf
Copy link
Member

@fanf fanf commented Jun 7, 2021

https://issues.rudder.io/issues/19229

So, I wasn't able to reproduce the problem, and that PR is a bit of testing from what I know can lead to that given the thread dump @ncharles got.

My best guess is that the lazy val in the object + runNow is the root of the problem. We had that problem (hard to debug) in other tickets (https://issues.rudder.io/issues/17228).

So: change the lazy val for cache.

There was other problems in the services init that are unlikely to be linked to the deadlock, but who knows?

  • don't use hardcoded path (moreover, it can be a problem in security audit), use them in class parameters;
  • there was a class for whose package declaration and directory tree was not matching, which is known to lead to strange things in the JVM/Classloaders.
  • there was duplicated resources (in toserve/branding and branding/toserve),

@fanf fanf requested a review from ncharles June 7, 2021 13:23

private[this] lazy val cache : Ref[Either[RudderError, BrandingConf]] = (for {
private[this] val cache : Ref[Either[RudderError, BrandingConf]] = (for {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is likely the culprit

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder-plugins/pull/380
-- Your faithful QA
Kant merge: "Morality is not the doctrine of how we may make ourselves happy, but how we may make ourselves worthy of happiness."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/40141/console)

@fanf
Copy link
Member Author

fanf commented Jun 10, 2021

OK, merging this PR

@fanf fanf merged commit f62ec61 into Normation:branches/rudder/6.1 Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants