Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!

Bug 808845

Summary: net-vpn/i2pd: Does not remove PID file upon exit
Product: Gentoo Linux Reporter: PF4Public <PF4Public>
Component: Current packagesAssignee: Alexey Korepanov <kaikaikai>
Status: UNCONFIRMED ---    
Severity: normal CC: klondike, proxy-maint, sam
Priority: Normal Keywords: PullRequest
Version: unspecified   
Hardware: All   
OS: Linux   
See Also: https://github.com/gentoo/gentoo/pull/22289
Whiteboard:
Package list:
Runtime testing required: ---

Description PF4Public 2021-08-18 07:33:45 UTC
After i2pd was stopped, it's PID file isn't removed, therefore logrotate tries to HUP it and fails.

Reproducible: Always
Comment 1 Alexey Korepanov 2021-09-10 11:14:48 UTC
I can't reproduce... @PF4Public, can you give any details? like what init system do you use, how did you stop i2pd?

I imagine this can be a pretty bad problem if i2pd crashes or is otherwise improperly stopped, its PID is reused for something else, but logrotate is still regularly sending HUP.

I'm not quite sure how to check withing the logrotate script that the pid file is still valid and corresponds to our i2pd service.
Comment 2 PF4Public 2021-09-10 15:35:24 UTC
I use OpenRC:

dellgen ~ # /etc/init.d/i2pd start
i2pd                 | * /var/log/i2pd.log: creating file
i2pd                 | * /var/log/i2pd.log: correcting owner
i2pd                 | * /run/i2pd: creating directory
i2pd                 | * /run/i2pd: correcting owner
i2pd                 | * Starting i2pd ...                            [ ok ]
dellgen ~ # ls -la /run/i2pd/i2pd.pid 
-rw------- 1 i2pd i2pd 4 10. Sep 18:31 /run/i2pd/i2pd.pid
dellgen ~ # /etc/init.d/i2pd stop
i2pd                 | * Stopping i2pd ...                            [ ok ]
dellgen ~ # ls -la /run/i2pd/i2pd.pid 
-rw------- 1 i2pd i2pd 4 10. Sep 18:31 /run/i2pd/i2pd.pid
Comment 3 Alexey Korepanov 2021-09-10 18:05:21 UTC
@FP4Public, can you test this openrc script?

I only added pidfile="${I2PD_PID}" and stop_post, nothing else changed.

Would be interesting to know if the pidfile is removed when i2pd is stopped normally, and if i2pd is killed forcefully by say killall -9 i2pd

#!/sbin/openrc-run
# Copyright 1999-2016 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2

description="C++ daemon for accessing the I2P network"
description_graceful="Graceful shutdown, takes 10 minutes"

command="/usr/bin/i2pd"
command_args="${I2PD_OPTIONS}"
pidfile="${I2PD_PID}"
user="${I2PD_USER}:${I2PD_GROUP}"
start_stop_daemon_args="
    --user \"${user}\"
    --pidfile \"${I2PD_PID}\"
    --progress
"
retry="SIGTERM/20/SIGKILL/20"

I2PD_PID_DIR=$(dirname "${I2PD_PID}")

extra_started_commands="graceful"

depend() {
    use dns logger netmount
}

start_pre() {
    if [ -z "${I2PD_USER}" ] || \
       [ -z "${I2PD_GROUP}" ] || \
       [ -z "${I2PD_PID}" ] || \
       [ -z "${I2PD_LOG}" ] || \
       [ -z "${I2PD_OPTIONS}" ] ; then
        eerror "Not all variables I2PD_USER, I2PD_GROUP, I2PD_PID, I2PD_OPTIONS, I2PD_LOG are defined."
        eerror "Check your /etc/conf.d/i2pd."
        return 1
    fi
    checkpath -f -o "${user}" "${I2PD_LOG}"
    checkpath -d -m 0750 -o "${user}" "${I2PD_PID_DIR}"
}

stop_post() {
    rm -f "${I2PD_PID}"
}

graceful() {
    # on SIGINT, i2pd stops accepting tunnels and shuts down in 600 seconds
    ebegin "Gracefully stopping i2pd, this takes 10 minutes"
    mark_service_stopping
    eval start-stop-daemon --stop ${start_stop_daemon_args} \
        --exec "${command}" --retry 'SIGINT/620/SIGTERM/20/SIGKILL/20'
    eend $? && mark_service_stopped
}
Comment 4 Alexey Korepanov 2021-09-10 19:25:35 UTC
Another option would be to rewrite i2pd init files so that i2pd is started in foreground and managed my supervise-daemon:

>>> cat files/i2pd-2.39.0.confd 
I2PD_USER=i2pd
I2PD_GROUP=i2pd
I2PD_LOG=/var/log/i2pd.log

# max number of open files (for floodfill)
rc_ulimit="-n 4096"

# Options to i2pd
I2PD_OPTIONS="--service \
--log=file --logfile=${I2PD_LOG} \
--conf=/etc/i2pd/i2pd.conf --tunconf=/etc/i2pd/tunnels.conf"


>>> cat files/i2pd-2.39.0.initd 
#!/sbin/openrc-run
# Copyright 1999-2021 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2

description="C++ daemon for accessing the I2P network"
description_graceful="Graceful shutdown, takes 10 minutes"

supervisor="supervise-daemon"
command="/usr/bin/i2pd"
command_args="${I2PD_OPTIONS}"
pidfile="/run/i2pd.pid"
user="${I2PD_USER}:${I2PD_GROUP}"
retry="SIGTERM/20/SIGKILL/20"

extra_started_commands="graceful"

depend() {
    use dns logger netmount
}

start_pre() {
    if [ -z "${I2PD_USER}" ] || \
       [ -z "${I2PD_GROUP}" ] || \
       [ -z "${I2PD_LOG}" ] || \
       [ -z "${I2PD_OPTIONS}" ] ; then
        eerror "Not all variables I2PD_USER, I2PD_GROUP, I2PD_PID, I2PD_OPTIONS, I2PD_LOG are defined."
        eerror "Check your /etc/conf.d/i2pd."
        return 1
    fi
    checkpath -f -o "${user}" "${I2PD_LOG}"
}

graceful() {
    # on SIGINT, i2pd stops accepting tunnels and shuts down in 600 seconds
    ebegin "Gracefully stopping i2pd, this takes 10 minutes"
    mark_service_stopping
    eval ${supervisor} ${RC_SVCNAME} --stop --pidfile "${pidfile}" \
        --retry 'SIGINT/620/SIGTERM/20/SIGKILL/20'
    eend $? && mark_service_stopped
}


@PF4Public it would be really great if you could test it
Comment 5 Alexey Korepanov 2021-09-14 08:36:39 UTC
I added a simple fix to the regular version bump
https://github.com/gentoo/gentoo/pull/22289

But it does not solve the problem if i2pd crashes.
The above scripts with supervise-daemon might solve that, but they need to be tested. I don't have a gentoo box with openrc at the moment.
Comment 6 PF4Public 2021-09-15 17:39:20 UTC
Hi Alexey
Sorry for the delay: I was heavily occupied by my offline activities.

I've tested the simple fix (as it is in your comment) and it indeed clears the PID file upon exit. But, like you've suspected, crashing i2pd keeps the PID:

dellgen ~ # /etc/init.d/i2pd start
i2pd                 | * Starting i2pd ...                                      [ ok ]
dellgen ~ # killall -9 i2pd
dellgen ~ # /etc/init.d/i2pd status
 * status: crashed
dellgen ~ # ls -la /run/i2pd/i2pd.pid 
-rw------- 1 i2pd i2pd 6 15. Sep 20:13 /run/i2pd/i2pd.pid

I've also tested the second solution from your second comment and not only it clears the PID file after exit, but it also restarts the daemon after it had been crashed (by killall -9 i2pd). So, I'd call it improvement! Would be good to have it along with version bump!

Note though, that in your comment pidfile is set to "/run/i2pd.pid", while it was created as "/run/i2pd/i2pd.pid" earlier. That was either a mistype or your intention — don't know, just mentioning it.
Comment 7 Alexey Korepanov 2021-09-15 18:01:10 UTC
Thanks for testing!

I added a commit to the PR on github. The change from /run/i2pd/i2pd.pid to /run/i2pd.pid is intended. When the pid file is not managed by the service (which is the right way) we don't need a folder for pid where the daemon can write.
Comment 8 Larry the Git Cow gentoo-dev 2021-09-21 07:03:31 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=1c0d7d298402386bf4ac8dd298b22a127c988008

commit 1c0d7d298402386bf4ac8dd298b22a127c988008
Author:     Alexey Korepanov <kaikaikai@yandex.ru>
AuthorDate: 2021-09-14 08:22:08 +0000
Commit:     Joonas Niilola <juippis@gentoo.org>
CommitDate: 2021-09-21 07:03:04 +0000

    net-vpn/i2pd: version bump 2.39.0
    
    Closes: https://bugs.gentoo.org/812843
    Bug: https://bugs.gentoo.org/808845
    Signed-off-by: Alexey Korepanov <kaikaikai@yandex.ru>
    Closes: https://github.com/gentoo/gentoo/pull/22289
    Signed-off-by: Joonas Niilola <juippis@gentoo.org>

 net-vpn/i2pd/Manifest                              |  2 +-
 net-vpn/i2pd/files/i2pd-2.25.0-lib-path.patch      | 22 ----------------------
 .../{i2pd-2.6.0-r3.initd => i2pd-2.39.0.initd}     |  7 ++++++-
 .../{i2pd-2.38.0-r4.ebuild => i2pd-2.39.0.ebuild}  |  6 +-----
 4 files changed, 8 insertions(+), 29 deletions(-)