NetBSD Problem Report #54661

From tgl@nuc1.sss.pgh.pa.us  Wed Oct 30 06:08:25 2019
Return-Path: <tgl@nuc1.sss.pgh.pa.us>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id BF9BF7A20C
	for <gnats-bugs@gnats.NetBSD.org>; Wed, 30 Oct 2019 06:08:25 +0000 (UTC)
Message-Id: <20191029214229.A66E218E5BA@nuc1.sss.pgh.pa.us>
Date: Tue, 29 Oct 2019 17:42:29 -0400 (EDT)
From: tgl@sss.pgh.pa.us
Reply-To: tgl@sss.pgh.pa.us
To: gnats-bugs@NetBSD.org
Subject: pthread_self() returns invalid value in some circumstances
X-Send-Pr-Version: 3.95

>Number:         54661
>Category:       lib
>Synopsis:       pthread_self() returns invalid value in some circumstances
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Oct 30 06:10:00 +0000 2019
>Last-Modified:  Wed Oct 30 20:00:01 +0000 2019
>Originator:     tgl@sss.pgh.pa.us
>Release:        NetBSD 8.1
>Environment:
System: NetBSD nuc1.sss.pgh.pa.us 8.1 NetBSD 8.1 (GENERIC) #0: Fri May 31 08:43:59 UTC 2019 mkrepro@mkrepro.NetBSD.org:/usr/src/sys/arch/amd64/compile/GENERIC amd64
Architecture: x86_64
Machine: amd64
>Description:
pthread_self() sometimes returns an invalid pointer ((void *) -1),
as illustrated in How-To-Repeat.  Now, it could be argued that the
failing example is pilot error because I didn't link with -lpthread.
The reason I think this is a bug to be fixed is that the problem also
arises in a case where a dynamically loadable library links with
-lpthread and uses pthread_self(), but it is loaded into a main
executable that does neither.  Neither component is violating any
man page requirement, but nonetheless the library sees behavior
that is clearly contrary to both POSIX (which says that pthread_self
shall not fail, full stop) and NetBSD's own pthread_self man page,
which mentions nothing of such corner-case requirements.

For context, the given test case works as I expect, with or without
-lpthread, on FreeBSD 11.0, OpenBSD 6.4, and Fedora 30.  The real-world
use case underlying this is that libpython 3.7 fails when loaded into
postgresql on NetBSD 8.0 or 8.1, as discussed at

https://www.postgresql.org/message-id/4344.1572380716%40sss.pgh.pa.us

>How-To-Repeat:
$ cat test.c
/* this is stdio.h and pthread.h, not sure if send-pr will break the code */
#include <stdio.h>
#include <pthread.h>

int main()
{
  pthread_t id = pthread_self();

  printf("self = %p\n", id);
  return 0;
}
$ gcc test.c
$ ./a.out
self = 0xffffffffffffffff
$ gcc test.c -lpthread
$ ./a.out
self = 0x754ae5a2b800

I argue that I should get a valid result either way.

>Fix:
I haven't looked at the code, but I surmise that pthread_self()
per se is in libc (else this test case would fail at link time)
but it returns a bogus value unless libpthread initialized some
static storage, and that initialization fails to happen unless
libpthread was loaded *at program start*.

A minimum fix would be to arrange for said initialization to
happen when libpthread is loaded, even if that happens dynamically
as a result of dlopen().  That would be enough to solve my immediate
use case.  I note though that other modern systems seem to manage
to return a valid value even if libpthread is never loaded.
It's arguable that that's what POSIX requires: if the function is
there, it's not supposed to fail.

>Audit-Trail:
From: Joerg Sonnenberger <joerg@bec.de>
To: gnats-bugs@netbsd.org
Cc: lib-bug-people@netbsd.org, gnats-admin@netbsd.org,
	netbsd-bugs@netbsd.org
Subject: Re: lib/54661: pthread_self() returns invalid value in some
 circumstances
Date: Wed, 30 Oct 2019 11:08:38 +0100

 On Wed, Oct 30, 2019 at 06:10:00AM +0000, tgl@sss.pgh.pa.us wrote:
 > pthread_self() sometimes returns an invalid pointer ((void *) -1),
 > as illustrated in How-To-Repeat.

 Why do you think it's an invalid pointer? It's an opaque value.
 Comparing it against any fixed value is a bug in the caller.

 Joerg

From: Joerg Sonnenberger <joerg@bec.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: gnats-bugs@netbsd.org, lib-bug-people@netbsd.org,
	netbsd-bugs@netbsd.org
Subject: Re: lib/54661: pthread_self() returns invalid value in some
 circumstances
Date: Wed, 30 Oct 2019 15:52:02 +0100

 On Wed, Oct 30, 2019 at 10:44:39AM -0400, Tom Lane wrote:
 > Joerg Sonnenberger <joerg@bec.de> writes:
 > > The following reply was made to PR lib/54661; it has been noted by GNATS.
 > >  On Wed, Oct 30, 2019 at 06:10:00AM +0000, tgl@sss.pgh.pa.us wrote:
 > >>> pthread_self() sometimes returns an invalid pointer ((void *) -1),
 > >>> as illustrated in How-To-Repeat.
 >  
 > >  Why do you think it's an invalid pointer? It's an opaque value.
 > >  Comparing it against any fixed value is a bug in the caller.
 > 
 > I tend to agree that Python shouldn't be checking for -1, but even
 > if it removed that check, this behavior would still be problematic,
 > because pthread_self() is returning a value that won't work correctly
 > in later thread operations.  For example, it will not compare equal
 > to the result of a later pthread_self() if somebody successfully
 > loads/initializes libpthread.so in between.
 > 
 > I've now looked at the relevant libpthread code, and what seems to be
 > happening is this: libpython.so is invoking libpthread's version of
 > pthread_self(), but that is just redirecting to the stub version in
 > libc, evidently because pthread__init hasn't been called.  So the
 > question is why not.  I see bug fixes in libpthread's CVS history
 > that claim to be fixing cases where libpthread is dlopen'd, but
 > somehow that's not working here.  Maybe the case where libpthread
 > is a dependency of a dlopen'd library isn't covered?
 > 
 > I also kind of wonder why the redirect-to-the-stubs behavior is
 > there at all.  Why not just forcibly do pthread__init, if it's not
 > been done already, when one of libpthread's functions is called?

 We didn't allow dynamic loading of libpthread at all for a long time. I
 still believe allowing it was a mistake, but *shrug*. We certainly do
 not support calling pthread_create if libpthread was not initially
 loaded. As much pthread_self() can only ever return the main thread.

 Joerg

From: Kamil Rytarowski <n54@gmx.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: lib/54661: pthread_self() returns invalid value in some
 circumstances
Date: Wed, 30 Oct 2019 15:58:41 +0100

 On 30.10.2019 15:55, Joerg Sonnenberger wrote:
 > The following reply was made to PR lib/54661; it has been noted by GNATS=
 .
 >
 > From: Joerg Sonnenberger <joerg@bec.de>
 > To: Tom Lane <tgl@sss.pgh.pa.us>
 > Cc: gnats-bugs@netbsd.org, lib-bug-people@netbsd.org,
 > 	netbsd-bugs@netbsd.org
 > Subject: Re: lib/54661: pthread_self() returns invalid value in some
 >  circumstances
 > Date: Wed, 30 Oct 2019 15:52:02 +0100
 >
 >  On Wed, Oct 30, 2019 at 10:44:39AM -0400, Tom Lane wrote:
 >  > Joerg Sonnenberger <joerg@bec.de> writes:
 >  > > The following reply was made to PR lib/54661; it has been noted by =
 GNATS.
 >  > >  On Wed, Oct 30, 2019 at 06:10:00AM +0000, tgl@sss.pgh.pa.us wrote:
 >  > >>> pthread_self() sometimes returns an invalid pointer ((void *) -1)=
 ,
 >  > >>> as illustrated in How-To-Repeat.
 >  >
 >  > >  Why do you think it's an invalid pointer? It's an opaque value.
 >  > >  Comparing it against any fixed value is a bug in the caller.
 >  >
 >  > I tend to agree that Python shouldn't be checking for -1, but even
 >  > if it removed that check, this behavior would still be problematic,
 >  > because pthread_self() is returning a value that won't work correctly
 >  > in later thread operations.  For example, it will not compare equal
 >  > to the result of a later pthread_self() if somebody successfully
 >  > loads/initializes libpthread.so in between.
 >  >
 >  > I've now looked at the relevant libpthread code, and what seems to be
 >  > happening is this: libpython.so is invoking libpthread's version of
 >  > pthread_self(), but that is just redirecting to the stub version in
 >  > libc, evidently because pthread__init hasn't been called.  So the
 >  > question is why not.  I see bug fixes in libpthread's CVS history
 >  > that claim to be fixing cases where libpthread is dlopen'd, but
 >  > somehow that's not working here.  Maybe the case where libpthread
 >  > is a dependency of a dlopen'd library isn't covered?
 >  >
 >  > I also kind of wonder why the redirect-to-the-stubs behavior is
 >  > there at all.  Why not just forcibly do pthread__init, if it's not
 >  > been done already, when one of libpthread's functions is called?
 >
 >  We didn't allow dynamic loading of libpthread at all for a long time. I
 >  still believe allowing it was a mistake, but *shrug*. We certainly do
 >  not support calling pthread_create if libpthread was not initially
 >  loaded. As much pthread_self() can only ever return the main thread.
 >
 >  Joerg
 >
 >

 I don't know the original use-case, but there is _lwp_self() that is
 guaranteed to work in the same way with and without libpthread.

From: Tom Lane <tgl@sss.pgh.pa.us>
To: gnats-bugs@netbsd.org, Joerg Sonnenberger <joerg@bec.de>
Cc: lib-bug-people@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: lib/54661: pthread_self() returns invalid value in some circumstances
Date: Wed, 30 Oct 2019 10:44:39 -0400

 Joerg Sonnenberger <joerg@bec.de> writes:
 > The following reply was made to PR lib/54661; it has been noted by GNATS.
 >  On Wed, Oct 30, 2019 at 06:10:00AM +0000, tgl@sss.pgh.pa.us wrote:
 >>> pthread_self() sometimes returns an invalid pointer ((void *) -1),
 >>> as illustrated in How-To-Repeat.

 >  Why do you think it's an invalid pointer? It's an opaque value.
 >  Comparing it against any fixed value is a bug in the caller.

 I tend to agree that Python shouldn't be checking for -1, but even
 if it removed that check, this behavior would still be problematic,
 because pthread_self() is returning a value that won't work correctly
 in later thread operations.  For example, it will not compare equal
 to the result of a later pthread_self() if somebody successfully
 loads/initializes libpthread.so in between.

 I've now looked at the relevant libpthread code, and what seems to be
 happening is this: libpython.so is invoking libpthread's version of
 pthread_self(), but that is just redirecting to the stub version in
 libc, evidently because pthread__init hasn't been called.  So the
 question is why not.  I see bug fixes in libpthread's CVS history
 that claim to be fixing cases where libpthread is dlopen'd, but
 somehow that's not working here.  Maybe the case where libpthread
 is a dependency of a dlopen'd library isn't covered?

 I also kind of wonder why the redirect-to-the-stubs behavior is
 there at all.  Why not just forcibly do pthread__init, if it's not
 been done already, when one of libpthread's functions is called?

 			regards, tom lane

From: Tom Lane <tgl@sss.pgh.pa.us>
To: gnats-bugs@netbsd.org, Joerg Sonnenberger <joerg@bec.de>
Cc: lib-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: lib/54661: pthread_self() returns invalid value in some circumstances
Date: Wed, 30 Oct 2019 11:05:05 -0400

 Joerg Sonnenberger <joerg@bec.de> writes:
 >  We didn't allow dynamic loading of libpthread at all for a long time. I
 >  still believe allowing it was a mistake, but *shrug*. We certainly do
 >  not support calling pthread_create if libpthread was not initially
 >  loaded. As much pthread_self() can only ever return the main thread.

 Hmm.  So the idea is to lock down libpthread.so to act like the stub
 functions, if it's dlopen'd after main() initialization?  Weird, but
 no skin off my nose --- from a Postgres-centric viewpoint, we don't
 support anybody trying to start additional threads inside a Python
 function, anyway.

 In that case we should close this as not-a-bug, and I'll go rant
 at the Python people that they shouldn't be making an error check
 on pthread_self's result.  Or at least, -1 is a really unfortunate
 choice of value for PYTHREAD_INVALID_THREAD_ID.

 			regards, tom lane

From: Jason Thorpe <thorpej@me.com>
To: Joerg Sonnenberger <joerg@bec.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>,
 gnats-bugs@netbsd.org,
 lib-bug-people@netbsd.org,
 netbsd-bugs@netbsd.org
Subject: Re: lib/54661: pthread_self() returns invalid value in some
 circumstances
Date: Wed, 30 Oct 2019 11:30:57 -0700

 > On Oct 30, 2019, at 7:52 AM, Joerg Sonnenberger <joerg@bec.de> wrote:
 > 
 > We didn't allow dynamic loading of libpthread at all for a long time. I
 > still believe allowing it was a mistake, but *shrug*. We certainly do
 > not support calling pthread_create if libpthread was not initially
 > loaded. As much pthread_self() can only ever return the main thread.

 Honestly, I think it's high time we merged libpthread into libc directly.

 -- thorpej

From: Kamil Rytarowski <n54@gmx.com>
To: 
Cc: gnats-bugs@netbsd.org
Subject: Re: lib/54661: pthread_self() returns invalid value in some
 circumstances
Date: Wed, 30 Oct 2019 20:34:17 +0100

 On 30.10.2019 19:30, Jason Thorpe wrote:
 >
 >> On Oct 30, 2019, at 7:52 AM, Joerg Sonnenberger <joerg@bec.de> wrote:
 >>
 >> We didn't allow dynamic loading of libpthread at all for a long time. I
 >> still believe allowing it was a mistake, but *shrug*. We certainly do
 >> not support calling pthread_create if libpthread was not initially
 >> loaded. As much pthread_self() can only ever return the main thread.
 >
 > Honestly, I think it's high time we merged libpthread into libc directly=
 .
 >
 > -- thorpej
 >

 I'm just noting that if we would merge libpthread into libc, we still
 need to keep a stub libpthread for c99 (POSIX).

From: Jason Thorpe <thorpej@me.com>
To: gnats-bugs@netbsd.org
Cc: lib-bug-people@netbsd.org,
 gnats-admin@netbsd.org,
 netbsd-bugs@netbsd.org,
 tgl@sss.pgh.pa.us
Subject: Re: lib/54661: pthread_self() returns invalid value in some
 circumstances
Date: Wed, 30 Oct 2019 12:55:02 -0700

 > On Oct 30, 2019, at 12:35 PM, Kamil Rytarowski <n54@gmx.com> wrote:

 > I'm just noting that if we would merge libpthread into libc, we still
 > need to keep a stub libpthread for c99 (POSIX).

 Yes, understood.

 -- thorpej

Home	PR Database Search