From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 6B6F11FF170
for <inbox@lore.proxmox.com>; Tue, 3 Dec 2024 10:14:31 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 2010E2ED34;
Tue, 3 Dec 2024 10:14:37 +0100 (CET)
Message-ID: <f568b9f6-fd6e-468e-9c61-a14bb3064675@proxmox.com>
Date: Tue, 3 Dec 2024 10:14:34 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird Beta
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
Proxmox Backup Server development discussion <pbs-devel@lists.proxmox.com>
References: <20241129153744.4128441-1-d.csapak@proxmox.com>
<4e26505b-2348-4132-b8ac-39026d02b659@proxmox.com>
Content-Language: en-US
From: Dominik Csapak <d.csapak@proxmox.com>
In-Reply-To: <4e26505b-2348-4132-b8ac-39026d02b659@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.016 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [server.rs]
Subject: Re: [pbs-devel] [PATCH proxmox] daemon: clean up middle process of
double fork
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
<pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>
On 12/2/24 17:47, Thomas Lamprecht wrote:
> Am 29.11.24 um 16:37 schrieb Dominik Csapak:
>> so we don't leave around a zombie process when the old daemon still
>> needs to run, because of e.g. a running task.
>>
>> Since this is mostly a cosmetic issue though, only try a clean up
>> once, so we don't unnecessarily block or run into other issues here.
>> (It could happen that it didn't exit at that point, but it's very
>> unlikely.)
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> maybe the comment could be improved, but i tried to be not overly
>> verbose there, since it's not really an issue anyway
>>
>> proxmox-daemon/src/server.rs | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/proxmox-daemon/src/server.rs b/proxmox-daemon/src/server.rs
>> index efea9078..edc64795 100644
>> --- a/proxmox-daemon/src/server.rs
>> +++ b/proxmox-daemon/src/server.rs
>> @@ -165,10 +165,12 @@ impl Reloader {
>> // No matter how we managed to get here, this is the time where we bail out quickly:
>> unsafe { libc::_exit(-1) }
>> }
>> - Ok(ForkResult::Parent { child }) => {
>> + Ok(ForkResult::Parent {
>> + child: middle_child,
>> + }) => {
>> log::debug!(
>> "forked off a new server (first pid: {}), waiting for 2nd pid",
>> - child
>> + middle_child
>> );
>> std::mem::drop(pnew);
>> let mut pold = std::fs::File::from(pold);
>> @@ -211,6 +213,13 @@ impl Reloader {
>> log::error!("child vanished during reload: {}", e);
>> }
>>
>> + // try exactly once to get rid of the zombie process of middle_child, but
>> + // non blocking and without error handling, since it's just cosmetic
>> + let _ = nix::sys::wait::waitpid(
>> + middle_child,
>> + Some(nix::sys::wait::WaitPidFlag::WNOHANG),
>> + );
>
> why not blocking though? If that does not work something would be seriously
> wrong. But not _that_ hard feelings, as long as the old process exits this
> will be cleaned up by systemd anyway, but I really would like to have some
> error handling here, as that definitively can only help.
my fear was that if there's something wrong with the middle child (e.g. something hangs)
we'll never close the parent process either and have two old processes hanging around instead of one.
but yes (also as fabian said), logging the error here at least would be good
i'll send a v2
>
>> +
>> Ok(())
>> }
>> Err(e) => {
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel