Skip to content

Commit

Permalink
Make request->child_pid into request->thread_id
Browse files Browse the repository at this point in the history 
There's no reason to use a pthread ID.  We can just expose
the thread number.  This makes it easier to debug threading issues
  • Loading branch information
@alandekok
alandekok committed Oct 5, 2013
1 parent 6a2d7a7 commit 092da2c
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 37 deletions.
2 changes: 1 addition & 1 deletion src/include/radiusd.h
View file
Expand Up @@ -212,7 +212,7 @@ struct auth_req {
request_data_t *data;
RADCLIENT *client;
#ifdef HAVE_PTHREAD_H
pthread_t child_pid;
int thread_id;
#endif
time_t timestamp;
unsigned int number; /* internal server number */
Expand Down
35 changes: 15 additions & 20 deletions src/main/event.c
View file
Expand Up @@ -74,7 +74,7 @@ static pthread_mutex_t proxy_mutex;
#define PTHREAD_MUTEX_LOCK if (have_children) pthread_mutex_lock
#define PTHREAD_MUTEX_UNLOCK if (have_children) pthread_mutex_unlock

static pthread_t NO_SUCH_CHILD_PID;
#define NO_CHILD_THREAD (0)
#else
/*
* This is easier than ifdef's throughout the code.
Expand Down Expand Up @@ -499,7 +499,7 @@ static void wait_for_child_to_die(void *ctx)
*/
if ((request->child_state == REQUEST_QUEUED) ||
((request->child_state == REQUEST_RUNNING) &&
(pthread_equal(request->child_pid, NO_SUCH_CHILD_PID) == 0))) {
(request->thread_id == NO_CHILD_THREAD))) {

/*
* Cap delay at max_request_time
Expand Down Expand Up @@ -1237,9 +1237,9 @@ static void wait_a_bit(void *ctx)
* the request.
*/
if (have_children &&
(pthread_equal(request->child_pid, NO_SUCH_CHILD_PID) == 0)) {
radlog(L_ERR, "WARNING: Unresponsive child for request %u, in component %s module %s",
request->number,
(request->thread_id == NO_CHILD_THREAD)) {
radlog(L_ERR, "WARNING: Unresponsive thread %d for request %u, in component %s module %s",
request->number, request->thread_id,
request->component ? request->component : "<server core>",
request->module ? request->module : "<server core>");

Expand All @@ -1258,7 +1258,7 @@ static void wait_a_bit(void *ctx)
case REQUEST_DONE:
done:
#ifdef HAVE_PTHREAD_H
request->child_pid = NO_SUCH_CHILD_PID;
request->thread_id = NO_CHILD_THREAD;
#endif

#ifdef WITH_COA
Expand All @@ -1285,7 +1285,7 @@ static void wait_a_bit(void *ctx)
case REQUEST_REJECT_DELAY:
case REQUEST_CLEANUP_DELAY:
#ifdef HAVE_PTHREAD_H
request->child_pid = NO_SUCH_CHILD_PID;
request->thread_id = NO_CHILD_THREAD;
#endif
request_stats_final(request);
/* FALL-THROUGH */
Expand Down Expand Up @@ -1697,7 +1697,7 @@ static int originated_coa_request(REQUEST *request)
DEBUG_PACKET(request, request->proxy, 1);

#ifdef HAVE_PTHREAD_H
request->child_pid = NO_SUCH_CHILD_PID;
request->thread_id = NO_CHILD_THREAD;
#endif

request->proxy_listener->send(request->proxy_listener,
Expand Down Expand Up @@ -1800,7 +1800,7 @@ static int process_proxy_reply(REQUEST *request)
/* FIXME: debug print stuff */
request->child_state = REQUEST_DONE;
#ifdef HAVE_PTHREAD_H
request->child_pid = NO_SUCH_CHILD_PID;
request->thread_id = NO_CHILD_THREAD;
#endif
return 0;
}
Expand Down Expand Up @@ -1870,7 +1870,7 @@ static int request_pre_handler(REQUEST *request)
request->reply->offset = -2; /* bad authenticator */
request->child_state = REQUEST_DONE;
#ifdef HAVE_PTHREAD_H
request->child_pid = NO_SUCH_CHILD_PID;
request->thread_id = NO_CHILD_THREAD;
#endif
return 0;
}
Expand Down Expand Up @@ -1950,7 +1950,7 @@ static int proxy_request(REQUEST *request)
request->num_proxied_requests = 1;
request->num_proxied_responses = 0;
#ifdef HAVE_PTHREAD_H
request->child_pid = NO_SUCH_CHILD_PID;
request->thread_id = NO_CHILD_THREAD;
#endif
request->child_state = REQUEST_PROXIED;

Expand Down Expand Up @@ -2390,7 +2390,7 @@ static void request_post_handler(REQUEST *request)
if (request->packet->dst_port == 0) {
/* FIXME: RDEBUG going to the next request */
#ifdef HAVE_PTHREAD_H
request->child_pid = NO_SUCH_CHILD_PID;
request->thread_id = NO_CHILD_THREAD;
#endif
request->child_state = REQUEST_DONE;
return;
Expand Down Expand Up @@ -2471,7 +2471,7 @@ static void request_post_handler(REQUEST *request)
request->next_when = when;
request->next_callback = reject_delay;
#ifdef HAVE_PTHREAD_H
request->child_pid = NO_SUCH_CHILD_PID;
request->thread_id = NO_CHILD_THREAD;
#endif
request->child_state = REQUEST_REJECT_DELAY;
return;
Expand Down Expand Up @@ -2588,7 +2588,7 @@ static void request_post_handler(REQUEST *request)

if (have_children) {
#ifdef HAVE_PTHREAD_H
request->child_pid = NO_SUCH_CHILD_PID;
request->thread_id = NO_CHILD_THREAD;
#endif
} else {
/*
Expand Down Expand Up @@ -2971,7 +2971,7 @@ int received_request(rad_listen_t *listener,
request->number = request_num_counter++;
request->priority = listener->type;
#ifdef HAVE_PTHREAD_H
request->child_pid = NO_SUCH_CHILD_PID;
request->thread_id = NO_CHILD_THREAD;
#endif

/*
Expand Down Expand Up @@ -3598,11 +3598,6 @@ int radius_event_init(CONF_SECTION *cs, int spawn_flag)
#endif

#ifdef HAVE_PTHREAD_H
#ifndef __MINGW32__
NO_SUCH_CHILD_PID = (pthread_t ) (0);
#else
NO_SUCH_CHILD_PID = pthread_self(); /* not a child thread */
#endif
/*
* Initialize the threads ONLY if we're spawning, AND
* we're running normally.
Expand Down
30 changes: 15 additions & 15 deletions src/main/threads.c
View file
Expand Up @@ -79,7 +79,7 @@ RCSID("$Id$")
* the current thread.
*
* pthread_id pthread id
* thread_num server thread number, 1...number of threads
* thread_id server thread number, 1...number of threads
* semaphore used to block the thread until a request comes in
* status is the thread running or exited?
* request_count the number of requests that this thread has handled
Expand All @@ -89,7 +89,7 @@ typedef struct THREAD_HANDLE {
struct THREAD_HANDLE *prev;
struct THREAD_HANDLE *next;
pthread_t pthread_id;
int thread_num;
int thread_id;
int status;
unsigned int request_count;
time_t timestamp;
Expand Down Expand Up @@ -123,7 +123,7 @@ typedef struct THREAD_POOL {
int active_threads; /* protected by queue_mutex */
int exited_threads;
int total_threads;
int max_thread_num;
int max_thread_id;
int start_threads;
int max_threads;
int min_spare_threads;
Expand Down Expand Up @@ -505,7 +505,7 @@ static void *request_handler_thread(void *arg)
* Wait to be signalled.
*/
DEBUG2("Thread %d waiting to be assigned a request",
self->thread_num);
self->thread_id);
re_wait:
if (sem_wait(&thread_pool.semaphore) != 0) {
/*
Expand All @@ -514,15 +514,15 @@ static void *request_handler_thread(void *arg)
* text.
*/
if (errno == EINTR) {
DEBUG2("Re-wait %d", self->thread_num);
DEBUG2("Re-wait %d", self->thread_id);
goto re_wait;
}
radlog(L_ERR, "Thread %d failed waiting for semaphore: %s: Exiting\n",
self->thread_num, strerror(errno));
self->thread_id, strerror(errno));
break;
}

DEBUG2("Thread %d got semaphore", self->thread_num);
DEBUG2("Thread %d got semaphore", self->thread_id);

#ifdef HAVE_OPENSSL_ERR_H
/*
Expand All @@ -545,11 +545,11 @@ static void *request_handler_thread(void *arg)
*/
if (!request_dequeue(&self->request, &fun)) continue;

self->request->child_pid = self->pthread_id;
self->request->thread_id = self->thread_id;
self->request_count++;

DEBUG2("Thread %d handling request %d, (%d handled so far)",
self->thread_num, self->request->number,
self->thread_id, self->request->number,
self->request_count);

self->request->module = "";
Expand All @@ -571,12 +571,12 @@ static void *request_handler_thread(void *arg)
if ((thread_pool.max_requests_per_thread > 0) &&
(self->request_count >= thread_pool.max_requests_per_thread)) {
DEBUG2("Thread %d handled too many requests",
self->thread_num);
self->thread_id);
break;
}
} while (self->status != THREAD_CANCELLED);

DEBUG2("Thread %d exiting...", self->thread_num);
DEBUG2("Thread %d exiting...", self->thread_id);

#ifdef HAVE_OPENSSL_ERR_H
/*
Expand Down Expand Up @@ -614,7 +614,7 @@ static void delete_thread(THREAD_HANDLE *handle)

rad_assert(handle->request == NULL);

DEBUG2("Deleting thread %d", handle->thread_num);
DEBUG2("Deleting thread %d", handle->thread_id);

prev = handle->prev;
next = handle->next;
Expand Down Expand Up @@ -671,7 +671,7 @@ static THREAD_HANDLE *spawn_thread(time_t now)
memset(handle, 0, sizeof(THREAD_HANDLE));
handle->prev = NULL;
handle->next = NULL;
handle->thread_num = thread_pool.max_thread_num++;
handle->thread_id = thread_pool.max_thread_id++;
handle->request_count = 0;
handle->status = THREAD_RUNNING;
handle->timestamp = time(NULL);
Expand All @@ -696,7 +696,7 @@ static THREAD_HANDLE *spawn_thread(time_t now)
*/
thread_pool.total_threads++;
DEBUG2("Thread spawned new child %d. Total threads in pool: %d",
handle->thread_num, thread_pool.total_threads);
handle->thread_id, thread_pool.total_threads);

/*
* Add the thread handle to the tail of the thread pool list.
Expand Down Expand Up @@ -783,7 +783,7 @@ int thread_pool_init(CONF_SECTION *cs, int *spawn_flag)
thread_pool.head = NULL;
thread_pool.tail = NULL;
thread_pool.total_threads = 0;
thread_pool.max_thread_num = 1;
thread_pool.max_thread_id = 1;
thread_pool.cleanup_delay = 5;
thread_pool.stop_flag = 0;
thread_pool.spawn_flag = *spawn_flag;
Expand Down
2 changes: 1 addition & 1 deletion src/main/util.c
View file
Expand Up @@ -414,7 +414,7 @@ REQUEST *request_alloc_fake(REQUEST *request)

fake->number = request->number;
#ifdef HAVE_PTHREAD_H
fake->child_pid = request->child_pid;
fake->thread_id = request->thread_id;
#endif
fake->parent = request;
fake->root = request->root;
Expand Down

0 comments on commit 092da2c

Please sign in to comment.