cancellation cleanup handling with linuxthreads

[Mike Frysinger <vapier at gentoo dot org>]

after poring over the IEEE spec for thread cancellation, i think i've tracked 
down a failure in linuxthreads' handling of clean up functions ... can anyone 
verify/correct me here ?  testing nptl doesn't show the bug i seem to be 
experiencing ...

the scenario i'm looking at isn't overly complicated ... two threads (MAIN and 
KILLME); the KILLME thread is cancellable and has registered cleanup 
functions which happen to call some functions which are cancellation 
endpoints.  the MAIN thread is enthusiastic and cancels KILLME multiple 
times.  the cleanup functions wrongly get called multiple times (!).

this part of the spec seems to be what linuxthreads is doing wrong:
http://www.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_09.html#tag_02_09_05_03
When a cancellation request is acted upon, or when a thread calls 
pthread_exit(), the thread first disables cancellation by setting its 
cancelability state to PTHREAD_CANCEL_DISABLE and its cancelability type to 
PTHREAD_CANCEL_DEFERRED.

in particular i'm looking at the top of join.c:__pthread_do_exit():
  /* Reset the cancellation flag to avoid looping if the cleanup handlers
     contain cancellation points */
  THREAD_SETMEM(self, p_canceled, 0);
that comment is wrong ?  perhaps the code should be:
  THREAD_SETMEM(self, p_cancelstate, PTHREAD_CANCEL_DISABLE);
  THREAD_SETMEM(self, p_canceltype, PTHREAD_CANCEL_DEFERRED);
that does fix my troubles ...

i'm using the attached sample code ... a good run has the application sleep 
forever while a bad run results in an assert failure ...
-mike

Attachment: pgp00000.pgp
Description: PGP signature

/*
 * This illustrates the bug where the cleanup function
 * of a thread may be called too many times.
 */

#define _GNU_SOURCE

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <assert.h>
#include <unistd.h>

#define warn(fmt, args...) fprintf(stderr, "[%p] " fmt, (void*)pthread_self(), ## args)
#define warnf(fmt, args...) warn("%s:%i: " fmt, __FUNCTION__, __LINE__, ## args)

int ok_to_kill_thread;

static void thread_killed(void *arg);

static void *KillMeThread(void *thread_par)
{
	pthread_t pthread_id;

	warnf("Starting %p\n", thread_par);

	pthread_id = pthread_self();
	pthread_cleanup_push(thread_killed, (void *)pthread_id);

	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
	pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);

	/* main code */
	warnf("please kill me now\n");
	while (1)
		ok_to_kill_thread = 1;

	pthread_cleanup_pop(0);
}

static void thread_killed(void *arg)
{
	static num_times_called = 0;

	warnf("killing %p [cnt=%i]\n", arg, ++num_times_called);
	assert(num_times_called == 1);

	/* pick any cancellation endpoint */
	while (1) {
		warnf("sleeping in cancellation endpoint ...\n");
		sleep(1);
	}

	warnf("done cleaning up\n");
}

int main()
{
	pthread_t app_pthread_id, evil_pthread_id;
	pthread_attr_t app_thread_attr;

	ok_to_kill_thread = 0;

	pthread_attr_init(&app_thread_attr);
	pthread_attr_setdetachstate(&app_thread_attr, PTHREAD_CREATE_DETACHED);
	pthread_attr_setscope(&app_thread_attr, PTHREAD_SCOPE_SYSTEM);
	pthread_attr_setinheritsched(&app_thread_attr, PTHREAD_EXPLICIT_SCHED);
	pthread_attr_setschedpolicy(&app_thread_attr, SCHED_OTHER);
	pthread_create(&app_pthread_id, &app_thread_attr, KillMeThread, NULL);

	warnf("waiting for thread to prepare itself\n");
	while (!ok_to_kill_thread)
		;

	while (1) {
		warnf("killing thread\n");
		pthread_cancel(app_pthread_id);
		sleep(5);
	}

	return 0;
}