public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: Dietmar Maurer <dietmar@proxmox.com>,
	Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [PATCH backup 5/7] proxy: implement 'reload-certificate' command
Date: Wed, 12 May 2021 10:00:55 +0200 (CEST)	[thread overview]
Message-ID: <1724370635.2142.1620806455083@webmail.proxmox.com> (raw)


> On 05/12/2021 9:42 AM Dietmar Maurer <dietmar@proxmox.com> wrote:
> 
>  
> Stupid questzioon, but why cant we do:
> 
> diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
> index fc773459..29298a22 100644
> --- a/src/bin/proxmox-backup-proxy.rs
> +++ b/src/bin/proxmox-backup-proxy.rs
> @@ -223,7 +223,6 @@ async fn accept_connection(
>      // Note that these must not be moved out/modified directly, they get pinned in the loop and
>      // "rearmed" after waking up:
>      let mut reload_tls = notify_tls_cert_reload.notified();
> -    let mut accept = listener.accept();
>  
>      loop {
>          let sock;
> @@ -231,7 +230,9 @@ async fn accept_connection(
>          // normally we'd use `tokio::pin!()` but we need this to happen outside the loop and we
>          // need to be able to "rearm" the futures:
>          let reload_tls_pin = unsafe { Pin::new_unchecked(&mut reload_tls) };
> -        let accept_pin = unsafe { Pin::new_unchecked(&mut accept) };
> +        //let accept_pin = unsafe { Pin::new_unchecked(&mut accept) };
> +        let accept = listener.accept();
> +

For the `accept` call this can even work, but not for the notify one
(won't compile), which is a bit weird, since looking up the Notify code,
it *does* have code in the Drop handler to forward wakeups in case it
gets dropped before being received, however, both need to have correct
code to handle this somehow, and I felt like dropping and requerying is
less "nice" and allows for more "weird" errors (similar to the
DuplexStream issue) happening, and I didn't feel comfortable leaving
more room for our main client accept loop to end up in an unexplained
"hanging" state somehow.
(I have trust issues sometimes...)
But sure, we could do this for accept.
(With the patch set I just sent out that would just need the Box::pin()
calls removed)

I wish there was some nice form of a `select_loop!`-like helper...




             reply	other threads:[~2021-05-12  8:01 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-12  8:00 Wolfgang Bumiller [this message]
  -- strict thread matches above, loose matches on Subject: below --
2021-05-12  9:17 Wolfgang Bumiller
2021-05-12  9:13 Dietmar Maurer
2021-05-12  9:01 Wolfgang Bumiller
2021-05-12  8:37 Dietmar Maurer
2021-05-12  7:42 Dietmar Maurer
2021-05-11 13:53 [pbs-devel] [PATCH backup 0/7] hot-reload proxy certificates Wolfgang Bumiller
2021-05-11 13:53 ` [pbs-devel] [PATCH backup 5/7] proxy: implement 'reload-certificate' command Wolfgang Bumiller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1724370635.2142.1620806455083@webmail.proxmox.com \
    --to=w.bumiller@proxmox.com \
    --cc=dietmar@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal