From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <dev-bounces@dpdk.org> Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 176F0455FE; Thu, 11 Jul 2024 17:32:43 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 065EC42FA1; Thu, 11 Jul 2024 17:32:43 +0200 (CEST) Received: from mail-pf1-f172.google.com (mail-pf1-f172.google.com [209.85.210.172]) by mails.dpdk.org (Postfix) with ESMTP id 7EE3A402D6 for <dev@dpdk.org>; Thu, 11 Jul 2024 17:32:42 +0200 (CEST) Received: by mail-pf1-f172.google.com with SMTP id d2e1a72fcca58-70aec66c936so893683b3a.0 for <dev@dpdk.org>; Thu, 11 Jul 2024 08:32:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1720711961; x=1721316761; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=YKDIpQ2nCrfZhcGu6pqT7HfUm4Vy2HlE/WJmC1mCAKI=; b=MshXd8nBjsZfg6WE1gokZiTS1nNQduWXB0GhIcyxlEK0K3kJRD8b2/TXsWFGiRO711 0Rncz9HS2yBAPobjdqn5dor0a3uQBGowSbrQ7fY+opPFizOAuINZrD8MhzEx+MFoAJky sP8txipnAD7zcRJHWjxl8NiNsL8jRGVgMRbZA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720711961; x=1721316761; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=YKDIpQ2nCrfZhcGu6pqT7HfUm4Vy2HlE/WJmC1mCAKI=; b=q7sKytBNxUVVmN8WJHaSymROB3vBnj/WmYCly+qP6d+JxHw+h+aBoqbMG6eMR9jD+j swBi6BBF0LuQpD12tovTDkKMXlSfCtZkyVSYeaiLb0LNnGxuco8aIz7WKcscZuI7H1vT PuWKZkPN08ltOi54g7Opj+wu4TC7j7gdNO3wNYp/6OOpi83ydyNO9uI+6S49U41n/EMu 2NFikZgOnH4dnTinF3n/tGAUcn2KYRmpJjFGnZIVmVbLQEwyzrxBSDOpEArJxOBe3T9x bDaqImMUlBjonSkXai8GxcwyLdIutVEVST5DVw1iUfDPqddlfmN0eH5hwHJjXpQ7ZHkZ m8Hg== X-Forwarded-Encrypted: i=1; AJvYcCW7X64BhtwGRejHStryALaVEOEiE8mqliVUW2u4d6LyGt2E7xnojmbqatmG4F4QaNq+6R82exQB4qEQO5w= X-Gm-Message-State: AOJu0YyPlqOQ4nrzOCR2hyxUZEUJ+z49upfjF/bmUTgz2d7sBLlI4fAI mOGw0lnsZq6nNE8gqPht85FzRWigFLhtWN4WFVmaMV7TWilvIEpwCMOq6hkDkwbtvtZKON0Rd5F hA9MzS/i4COHgOB30KziwzBNdddW6FXELTk+LaA== X-Google-Smtp-Source: AGHT+IHsG31XA9t/6e93VvgiA+c0MW2sXQblwj0yCjeAvhJaX0dV8Y7cf6L1IGyByuWPGODWsF7LlRaya/3PEOlhYlc= X-Received: by 2002:a05:6a00:1251:b0:706:700c:7864 with SMTP id d2e1a72fcca58-70b4351fde0mr11494554b3a.4.1720711961209; Thu, 11 Jul 2024 08:32:41 -0700 (PDT) MIME-Version: 1.0 References: <20240709163145.110030-1-jspewock@iol.unh.edu> <20240709163145.110030-3-jspewock@iol.unh.edu> <5d5804d7-156c-4321-83ec-f5230ccce664@pantheon.tech> In-Reply-To: <5d5804d7-156c-4321-83ec-f5230ccce664@pantheon.tech> From: Jeremy Spewock <jspewock@iol.unh.edu> Date: Thu, 11 Jul 2024 11:32:30 -0400 Message-ID: <CAAA20UQzxw_wjczj9fc+0xFwCsxmp8jxgkA0=CB1-Uicu03x8g@mail.gmail.com> Subject: Re: [PATCH v1 2/2] dts: improve starting and stopping interactive shells To: =?UTF-8?Q?Juraj_Linke=C5=A1?= <juraj.linkes@pantheon.tech> Cc: yoan.picchi@foss.arm.com, Honnappa.Nagarahalli@arm.com, thomas@monjalon.net, paul.szczepanek@arm.com, probb@iol.unh.edu, npratte@iol.unh.edu, Luca.Vizzarro@arm.com, wathsala.vithanage@arm.com, dev@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org On Thu, Jul 11, 2024 at 10:53=E2=80=AFAM Juraj Linke=C5=A1 <juraj.linkes@pantheon.tech> wrote: > > With the docs changes below, > Reviewed-by: Juraj Linke=C5=A1 <juraj.linkes@pantheon.tech> > > > diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/fr= amework/remote_session/interactive_shell.py > > index 11dc8a0643..fcaf1f6a5f 100644 > > --- a/dts/framework/remote_session/interactive_shell.py > > +++ b/dts/framework/remote_session/interactive_shell.py > > @@ -9,6 +9,9 @@ > > collection. > > """ > > > > +import weakref > > +from typing import ClassVar > > + > > from .single_active_interactive_shell import SingleActiveInteractiveS= hell > > > > > > @@ -16,18 +19,26 @@ class InteractiveShell(SingleActiveInteractiveShell= ): > > """Adds manual start and stop functionality to interactive shells= . > > > > Like its super-class, this class should not be instantiated direc= tly and should instead be > > - extended. This class also provides an option for automated cleanup= of the application through > > - the garbage collector. > > + extended. This class also provides an option for automated cleanup= of the application using a > > + weakref and a finalize class. This finalize class allows for clean= up of the class at the time > > + of garbage collection and also ensures that cleanup only happens o= nce. This way if a user > > + initiates the closing of the shell manually it is not repeated at = the time of garbage > > + collection. > > """ > > > > + _finalizer: weakref.finalize > > + #: Shells that do not require only one instance to be running shou= ldn't need more than 1 > > + #: attempt to start. > > This wording is a bit confusing. This could mean that the shells could > require multiple instances. We could try an alternative approach: > One attempt should be enough for shells which don't have to worry about > other instances closing before starting a new one. > > Or something similar. Good point, I'll make this change. > > > + _init_attempts: ClassVar[int] =3D 1 > > + > > def start_application(self) -> None: > > - """Start the application.""" > > + """Start the application. > > + > > + After the application has started, add a weakref finalize clas= s to manage cleanup. > > I think this could be improved to: > use :class:`weakref.finalize` to manage cleanup Good idea, referencing the actual class like this is more clear. > > > + """ > > self._start_application() > > + self._finalizer =3D weakref.finalize(self, self._close) > > > > def close(self) -> None: > > - """Properly free all resources.""" > > - self._close() > > - > > - def __del__(self) -> None: > > - """Make sure the session is properly closed before deleting th= e object.""" > > - self.close() > > + """Free all resources using finalize class.""" > > Also here: > using :class:`weakref.finalize` Ack. > > > + self._finalizer() >