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 9033A455FE;
	Thu, 11 Jul 2024 16:53:32 +0200 (CEST)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 711D4402D6;
	Thu, 11 Jul 2024 16:53:32 +0200 (CEST)
Received: from mail-ed1-f49.google.com (mail-ed1-f49.google.com
 [209.85.208.49]) by mails.dpdk.org (Postfix) with ESMTP id 0E0FE402D1
 for <dev@dpdk.org>; Thu, 11 Jul 2024 16:53:31 +0200 (CEST)
Received: by mail-ed1-f49.google.com with SMTP id
 4fb4d7f45d1cf-58b447c5112so1356106a12.3
 for <dev@dpdk.org>; Thu, 11 Jul 2024 07:53:31 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=pantheon.tech; s=google; t=1720709611; x=1721314411; darn=dpdk.org;
 h=content-transfer-encoding:in-reply-to:from:content-language
 :references:cc:to:subject:user-agent:mime-version:date:message-id
 :from:to:cc:subject:date:message-id:reply-to;
 bh=WDXLjYXafveo4O4/UkdTNg8X+e6DbnybPkutdXAPcW0=;
 b=r4JKnCfttUAQ5ESC/LJ/bbRUJnB9zr28/Oil3BCcSL0mr8gT7osC3nsLGSEyUri3mO
 GDsaR888xC0yu7CKtSZ7O27GmvQ5DTk2Ie+5+6dmb8yKF6B0y61rhcBwluV99Ii+VBEL
 rA4UYVPQ0coY4cbh+kRwfDv+kyjnWqN+OSw3SiH5DCNhFxSfmU58u0GbKHF/R3Hc1PGF
 5gBRGPsG29iHjLUnYG7K6PfNc9h/BGBTC3EuA3A7tbxmXm2vQbByEU/2jC2UYqv0pxCI
 czvd2eEqyn46Xh++scG5ZkHZ1IBr2H5iw17rYNxpLJdysDJocvLVPns+z4ehif5KaHL/
 Bxyg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20230601; t=1720709611; x=1721314411;
 h=content-transfer-encoding:in-reply-to:from:content-language
 :references:cc:to:subject:user-agent:mime-version:date:message-id
 :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;
 bh=WDXLjYXafveo4O4/UkdTNg8X+e6DbnybPkutdXAPcW0=;
 b=xGia98VgJHD3Hd+sEfNQGAFlLcwwumKh17KMAeeYlKH4M8absLnLGKrYMVjTGf58Bp
 GZELZTMjeg8+X+uXbvRh04+K4SsH2Q0FKcoKdIz/Ggm3vFadcANbq2hxh0uRdfWv0ALc
 vz1WmRSaCvRsgYjT9zEwmVJSnMJPylkYm3Lit5Iufcz66ugiPN2wh+f4rFzCqIOInQSQ
 MLihqzt3TGRFmVOf2TdQs+0ZigtSmQDDbVU6Cn825q+W2R0gj3vtwgo3Z8ImbfhH8ZpF
 ZdnXdB9apqe59mSi11UEWvqXf41X05aKMBwOASKc4uvGsukX04aTlB7On0B4RrMkMqgw
 xG1Q==
X-Gm-Message-State: AOJu0Yxe8yep7Utz/vvMFt/PTSk96AS3CGLJHN0AG3PXqIE7wiohzwZ2
 qEff24AzoAySXNcuFA9T5L8bNQmcjNxOjmzw0LL1uNvRu2oc5OTNV6ovKsKL7BA=
X-Google-Smtp-Source: AGHT+IHW5nuYaRbChfICXH8Y6VDEpS71wh0IZ17dkdTBTswKX79wRZnMuNHsK+DZm3/295btDoOlwg==
X-Received: by 2002:a05:6402:3587:b0:58b:c86:b230 with SMTP id
 4fb4d7f45d1cf-594bcab0351mr6448426a12.42.1720709610788; 
 Thu, 11 Jul 2024 07:53:30 -0700 (PDT)
Received: from [192.168.1.113] ([84.245.121.236])
 by smtp.gmail.com with ESMTPSA id
 4fb4d7f45d1cf-594bc4e80e8sm3544156a12.50.2024.07.11.07.53.29
 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);
 Thu, 11 Jul 2024 07:53:30 -0700 (PDT)
Message-ID: <5d5804d7-156c-4321-83ec-f5230ccce664@pantheon.tech>
Date: Thu, 11 Jul 2024 16:53:28 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1 2/2] dts: improve starting and stopping interactive
 shells
To: jspewock@iol.unh.edu, 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
Cc: dev@dpdk.org
References: <20240709163145.110030-1-jspewock@iol.unh.edu>
 <20240709163145.110030-3-jspewock@iol.unh.edu>
Content-Language: en-US
From: =?UTF-8?Q?Juraj_Linke=C5=A1?= <juraj.linkes@pantheon.tech>
In-Reply-To: <20240709163145.110030-3-jspewock@iol.unh.edu>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
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

With the docs changes below,
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

> diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/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 SingleActiveInteractiveShell
>   
>   
> @@ -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 directly 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 cleanup of the class at the time
> +    of garbage collection and also ensures that cleanup only happens once. 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 shouldn'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.

> +    _init_attempts: ClassVar[int] = 1
> +
>       def start_application(self) -> None:
> -        """Start the application."""
> +        """Start the application.
> +
> +        After the application has started, add a weakref finalize class to manage cleanup.

I think this could be improved to:
use :class:`weakref.finalize` to manage cleanup

> +        """
>           self._start_application()
> +        self._finalizer = 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 the object."""
> -        self.close()
> +        """Free all resources using finalize class."""

Also here:
using :class:`weakref.finalize`

> +        self._finalizer()