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

Be able to give lock to son processes detached by systemctl #367

Merged
merged 3 commits into from Oct 8, 2017

Conversation

alexAubin
Copy link
Member

@alexAubin alexAubin commented Sep 6, 2017

Problem

Following the changes in the lock system in Moulinette, it was noticed that it causes an issue for services that need to run yunohost command. Basically this kind of situation :

PID     PPID
  5     N/A     yunohost service foo start
  6      5      '-- systemctl foo start

 10     1       ./foo_start.sh
 11     10      '-- yunohost app foo setting bar -v meh

Process 10 is detached from process 6, therefore the lock taken by 5 cannot be used by 11...

This kind of situation happens at least in three different context :

  • Firewall reload at the end of postinstall
  • Firewall reload at any time from the yunohost admin
  • All (?) apps of Internet Cube (e.g. here)

Solution

The proposed solution is that, everytime we have to restart a service which needs to be able to run yunohost commands, we launch the service/systemctl command in background, get the PID of the command launched by systemctl (using systemctl show), and add a lock for it to be able to run yunohost commands. For this, we need YunoHost/moulinette#154

A flag is set in services.yml (need_lock: true) to define which services need the lock.

With this design, only the process currently having the lock (A) gives the lock to another process (B). A waits for B to ends its execution, hence there's no issue for concurrence since only one of these is active at a time. There's no possibility for a process C to grab the lock as long as A has it (except of course if C decide to mess with the lock file).

How to test

Run the following :

yunohost service stop yunohost-firewall --debug
yunohost service start yunohost-firewall --debug

A debug message should say that a lock was added for a given PID (except if the service is already stopped/started)

Validation

  • Principle agreement 1/2 : ljf
  • Quick review 1/1 : ljf
  • Simple test 0/1 :
  • Deep review 0/1? :

Copy link
Member

@zamentur zamentur left a comment

Choose a reason for hiding this comment

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

Principle agreement & quick review

@alexAubin alexAubin added this to the 2.7.x (fixes) or 2.8 milestone Oct 7, 2017
@alexAubin alexAubin merged commit 9903c48 into unstable Oct 8, 2017
@alexAubin alexAubin deleted the give-lock-to-systemctl-sons branch October 8, 2017 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants