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

Improve PID detection in pihole-FTL.service #2767

Merged
merged 2 commits into from Jun 1, 2019

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented May 30, 2019

By submitting this pull request, I confirm the following:
please fill any appropriate checkboxes, e.g: [X]

  • I have read and understood the contributors guide, as well as this entire template.
  • I have made only one major change in my proposed changes.
  • I have commented my proposed changes within the code.
  • I have tested my proposed changes, and have included unit tests where possible.
  • I am willing to help maintain this change if there are issues with it later.
  • I give this submission freely and claim no ownership.
  • It is compatible with the EUPL 1.2 license
  • I have squashed any insignificant commits. (git rebase)

Please make sure you Sign Off all commits. Pi-hole enforces the DCO.


What does this PR aim to accomplish?:
When dhcp-script is used, the parent process will stay active. This caused some of the service commands to malfunction. This PR fixes this issue. Thanks @RamSet for reporting this issue.

How does this PR accomplish the above?:
We try to obtain the PID from the PID file of pihole-FTL. If this is not possible, we fall back to the previously used method using pidof while, when multiple PIDs are returned by pidof, we select the last (=highest) PID.
This ensures that we send signals to the process that handles the signal appropriately.

What documentation changes (if any) are needed to support this PR?:
None

@DL6ER DL6ER added the Bug: fixed Contains a bug resolution label May 30, 2019
@DL6ER DL6ER added this to the v5.0 milestone May 30, 2019
@AzureMarker
Copy link
Contributor

Can we utilize the PID file?

AzureMarker
AzureMarker previously approved these changes May 31, 2019
Copy link
Contributor

@AzureMarker AzureMarker left a comment

Choose a reason for hiding this comment

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

In case we cannot use the PID file.

… is empty), fall back to using pidof + awk

Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER DL6ER changed the title Use last PID in case pidof returns multiple PIDs for pihole-FTL Improve PID detection in pihole-FTL.service May 31, 2019
@DL6ER
Copy link
Member Author

DL6ER commented May 31, 2019

@Mcat12 We utilize the PIDFILE now. The title and description have been corrected, accordingly.

@AzureMarker AzureMarker merged commit afd28fa into development Jun 1, 2019
@AzureMarker AzureMarker deleted the fix/multiple_pihole-FTL_PIDs branch June 1, 2019 05:03
@dschaper dschaper modified the milestones: v5.0, v4.3.2 Aug 20, 2019
@dschaper dschaper mentioned this pull request Aug 21, 2019
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-4-3-2-release-notes/23852/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: fixed Contains a bug resolution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants