[PATCH] Don't call init_thread_list in windows-nat.c

[PATCH] Don't call init_thread_list in windows-nat.c

[Tom Tromey]

I don't think there's any need to call init_thread_list in
windows-nat.c.  This patch removes it.  I tested this using the
internal AdaCore test suite on Windows, which FWIW does include some
multi-threaded inferiors.
---
 gdb/windows-nat.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 44c1950ace9..646ddda7328 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -401,7 +401,6 @@ static void
 windows_init_thread_list (void)
 {
   DEBUG_EVENTS ("called");
-  init_thread_list ();
   thread_list.clear ();
 }
 
-- 
2.34.1

Re: [PATCH] Don't call init_thread_list in windows-nat.c

[Pedro Alves]

On 2022-04-05 14:26, Tom Tromey via Gdb-patches wrote:
> I don't think there's any need to call init_thread_list in
> windows-nat.c.  This patch removes it.  I tested this using the
> internal AdaCore test suite on Windows, which FWIW does include some
> multi-threaded inferiors.

OK.

IIUC, thread_list is still a global after your globals elimination pass, so
that thread_list.clear() will also need some kind of adjustment for multi-inferior,
and I suspect it will just disappear too.

Pedro Alves

> ---
>  gdb/windows-nat.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
> index 44c1950ace9..646ddda7328 100644
> --- a/gdb/windows-nat.c
> +++ b/gdb/windows-nat.c
> @@ -401,7 +401,6 @@ static void
>  windows_init_thread_list (void)
>  {
>    DEBUG_EVENTS ("called");
> -  init_thread_list ();
>    thread_list.clear ();
>  }
>  

Re: [PATCH] Don't call init_thread_list in windows-nat.c

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> IIUC, thread_list is still a global after your globals elimination pass, so
Pedro> that thread_list.clear() will also need some kind of adjustment for multi-inferior,
Pedro> and I suspect it will just disappear too.

Yeah.  I only removed the globals (well, the removeable subset of them)
from gdb/nat/windows-nat.c, not from gdb/windows-nat.c.  For those I
think the idea should be to move them to a subclass of
private_inferior.  thread_list would be one of these.

Tom

Re: [PATCH] Don't call init_thread_list in windows-nat.c

[Simon Marchi]

On 2022-04-05 09:26, Tom Tromey via Gdb-patches wrote:
> I don't think there's any need to call init_thread_list in
> windows-nat.c.  This patch removes it.  I tested this using the
> internal AdaCore test suite on Windows, which FWIW does include some
> multi-threaded inferiors.
> ---
>  gdb/windows-nat.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
> index 44c1950ace9..646ddda7328 100644
> --- a/gdb/windows-nat.c
> +++ b/gdb/windows-nat.c
> @@ -401,7 +401,6 @@ static void
>  windows_init_thread_list (void)
>  {
>    DEBUG_EVENTS ("called");
> -  init_thread_list ();
>    thread_list.clear ();
>  }

So, init_thread_list clears all threads of all inferiors?

In multi-target and multi-inferiors scenarios, it just doesn't make
sense.  I quickly reviewed all calls to init_thread_list, and I think
they are all more or less bogus.  I think we could go even further and
get rid of it.

In windows-nat.c, it is called after creating a new inferior (run) and
attaching.  Since you are on the path of making windows-nat
multi-inferior aware, it definitely doesn't make sense to call that.
Otherwise, when running your second inferior, it would clear the threads
of your first inferior.

windows_init_thread_list also clears the thread_list global (which is
static to windows-nat.c).  I presume you will make that per-inferior
eventually?

Anyway, the patch LGTM as it is.

Simon

Re: [PATCH] Don't call init_thread_list in windows-nat.c

[Tom Tromey]

Simon> windows_init_thread_list also clears the thread_list global (which is
Simon> static to windows-nat.c).  I presume you will make that per-inferior
Simon> eventually?

Yeah, there are still a lot of globals in gdb/windows-nat.c that will
need to be per-inferior.

Tom