NetBSD Problem Report #49006

From www@NetBSD.org  Thu Jul 17 05:56:16 2014
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 54433A5D59
	for <gnats-bugs@gnats.NetBSD.org>; Thu, 17 Jul 2014 05:56:16 +0000 (UTC)
Message-Id: <20140717055614.C2C6EA655F@mollari.NetBSD.org>
Date: Thu, 17 Jul 2014 05:56:14 +0000 (UTC)
From: tony@develop-help.com
Reply-To: tony@develop-help.com
To: gnats-bugs@NetBSD.org
Subject: thread specific storage not always initialized to NULL
X-Send-Pr-Version: www-1.0

>Number:         49006
>Category:       lib
>Synopsis:       thread specific storage not always initialized to NULL
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    lib-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jul 17 06:00:00 +0000 2014
>Closed-Date:    Mon Nov 26 08:02:26 +0000 2018
>Last-Modified:  Mon Nov 26 08:02:26 +0000 2018
>Originator:     Tony Cook
>Release:        6.1.3
>Organization:
>Environment:
NetBSD neso.tony.develop-help.com 6.1.3 NetBSD 6.1.3 (GENERIC) amd64
>Description:
pthread_getspecific() will return non-NULL in some cases in a new thread.

If you:

a) create a key with pthread_key_create() with no desructor, 

b) create a thread which initializes the key to non-NULL,

c) join that thread, and

d) create a new thread,

that new thread will have non-NULL for the key.

>How-To-Repeat:
The output of the test program below should always be "result: 0x0".

neso$ cat pthread-specific.c
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>

pthread_key_t key;

int x; /* something to point to */

static void *
thread1(void *p) {
  pthread_setspecific(key, &x);

  return NULL;
}

static void *
thread2(void *p) {
  return pthread_getspecific(key);
}

static void
dest(void *p) { /* do nothing */ }

static void
fail(const char *msg) {
  fprintf(stderr, "%s\n", msg);
  exit(1);
}

int main() {
#ifdef HAVE_DESTRUCTOR
  int rs = pthread_key_create(&key, dest);
#else
  int rs = pthread_key_create(&key, NULL);
#endif
  if (rs)
    fail("pthread_key_create");

  void *result;
#ifndef ONE_THREAD
  pthread_t t1;
  if (pthread_create(&t1, NULL, thread1, NULL))
    fail("pthread_create(t1)");

  if (pthread_join(t1, &result))
    fail("pthread_join(t1)");
#endif

  pthread_t t2;
  if (pthread_create(&t2, NULL, thread2, NULL))
    fail("pthread_create(t2)");

  result = NULL;
  if (pthread_join(t2, &result))
    fail("pthread_join(t2)");

  printf("result: %p\n", result);

  return 0;
}
neso$ gcc -lpthread -opthread-specific pthread-specific.c 
neso$ ./pthread-specific 
result: 0x601508
neso$ gcc -lpthread -opthread-specific pthread-specific.c -DHAVE_DESTRUCTOR
neso$ ./pthread-specific 
result: 0x0
neso$ gcc -lpthread -opthread-specific pthread-specific.c -DONE_THREAD
neso$ ./pthread-specific 
result: 0x0
neso$ uname -a
NetBSD neso.tony.develop-help.com 6.1.3 NetBSD 6.1.3 (GENERIC) amd64

>Fix:

>Release-Note:

>Audit-Trail:
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/49006: thread specific storage not always initialized to NULL
Date: Thu, 17 Jul 2014 12:40:58 +0200

 This seems to work correctly in -current.

 Martin

From: Tony Cook <tony@develop-help.com>
To: gnats-bugs@NetBSD.org
Cc: lib-bug-people@NetBSD.org, gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org
Subject: Re: lib/49006: thread specific storage not always initialized to NULL
Date: Mon, 21 Jul 2014 10:09:22 +1000

 On Thu, Jul 17, 2014 at 10:45:00AM +0000, Martin Husemann wrote:
 > The following reply was made to PR lib/49006; it has been noted by GNATS.
 > 
 > From: Martin Husemann <martin@duskware.de>
 > To: gnats-bugs@NetBSD.org
 > Cc: 
 > Subject: Re: lib/49006: thread specific storage not always initialized to NULL
 > Date: Thu, 17 Jul 2014 12:40:58 +0200
 > 
 >  This seems to work correctly in -current.

 Looks like it was fixed in this chunk of v1.9 [1] of
 /lib/libpthread/pthread_tsd.c:

 @@ -206,17 +294,24 @@ pthread__destroy_tsd(pthread_t self)
  	do {
  		done = 1;
  		for (i = 0; i < PTHREAD_KEYS_MAX; i++) {
 -			if (self->pt_specific[i] != NULL) {
 -				pthread_mutex_lock(&tsd_mutex);
 +			struct pt_specific *pt = &self->pt_specific[i];
 +			if (pt->pts_next.ptqe_prev == NULL)
 +				continue;
 +			pthread_mutex_lock(&tsd_mutex);
 +
 +			if (pt->pts_next.ptqe_prev != NULL)  {
 +				PTQ_REMOVE(&pthread__tsd_list[i], pt, pts_next);
 +				val = pt->pts_value;
 +				pt->pts_value = NULL;
 +				pt->pts_next.ptqe_prev = NULL;
  				destructor = pthread__tsd_destructors[i];
 -				pthread_mutex_unlock(&tsd_mutex);
 -				if (destructor != NULL) {
 -					done = 0;
 -					val = self->pt_specific[i];
 -					/* See above */
 -					self->pt_specific[i] = NULL;
 -					(*destructor)(val);
 -				}
 +			} else
 +				destructor = NULL;
 +
 +			pthread_mutex_unlock(&tsd_mutex);
 +			if (destructor != NULL) {
 +				done = 0;
 +				(*destructor)(val);
  			}
  		}
  	} while (!done && iterations--);

 The removed code only set self->pt_specific[i] to NULL when the entry
 has a destructor, the new code doesn't have that flaw, assuming I
 understand it.

 Thanks,
 Tony

 [1] http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libpthread/pthread_tsd.c.diff?r1=1.8&r2=1.9&only_with_tag=MAIN

State-Changed-From-To: open->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Mon, 26 Nov 2018 08:02:26 +0000
State-Changed-Why:
This was fixed in -current before -7, and -6 is now EOL


>Unformatted:

Home	PR Database Search