Skip to content

Commit 69e8af5

Browse files
shintaro-iwasakihppritcha
authored andcommitted
mca/threads: fix tsd management
To suppress Valgrind warnings, opal_tsd_keys_destruct() needs to explicitly release TSD values of the main thread. However, they were not freed if keys are created by non-main threads. This patch fixes it. This patch also optimizes allocation of opal_tsd_key_values by doubling its size when count >= length instead of increasing the size by one. Signed-off-by: Shintaro Iwasaki <siwasaki@anl.gov>
1 parent 8cab081 commit 69e8af5

File tree

2 files changed

+30
-13
lines changed

2 files changed

+30
-13
lines changed

opal/mca/threads/argobots/threads_argobots_module.c

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,10 @@ struct opal_tsd_key_value {
3939
opal_tsd_destructor_t destructor;
4040
};
4141

42-
static ABT_thread opal_main_thread;
42+
static opal_mutex_t opal_tsd_lock = OPAL_MUTEX_STATIC_INIT;
4343
static struct opal_tsd_key_value *opal_tsd_key_values = NULL;
4444
static int opal_tsd_key_values_count = 0;
45+
static int opal_tsd_key_values_size = 0;
4546

4647
/*
4748
* Constructor
@@ -95,8 +96,6 @@ int opal_thread_join(opal_thread_t *t, void **thr_return)
9596

9697
void opal_thread_set_main()
9798
{
98-
opal_threads_argobots_ensure_init();
99-
opal_main_thread = opal_thread_get_argobots_self();
10099
}
101100

102101
int opal_thread_start(opal_thread_t *t)
@@ -125,14 +124,19 @@ int opal_tsd_key_create(opal_tsd_key_t *key, opal_tsd_destructor_t destructor)
125124
opal_threads_argobots_ensure_init();
126125
int rc;
127126
rc = ABT_key_create(destructor, key);
128-
if ((ABT_SUCCESS == rc) &&
129-
(opal_thread_get_argobots_self() == opal_main_thread)) {
130-
opal_tsd_key_values = (struct opal_tsd_key_value *)
131-
realloc(opal_tsd_key_values, (opal_tsd_key_values_count + 1) *
127+
if (ABT_SUCCESS == rc) {
128+
opal_mutex_lock(&opal_tsd_lock);
129+
if (opal_tsd_key_values_size <= opal_tsd_key_values_count) {
130+
opal_tsd_key_values_size = opal_tsd_key_values_size == 0
131+
? 1 : opal_tsd_key_values_size * 2;
132+
opal_tsd_key_values = (struct opal_tsd_key_value *)
133+
realloc(opal_tsd_key_values, opal_tsd_key_values_size *
132134
sizeof(struct opal_tsd_key_value));
135+
}
133136
opal_tsd_key_values[opal_tsd_key_values_count].key = *key;
134137
opal_tsd_key_values[opal_tsd_key_values_count].destructor = destructor;
135138
opal_tsd_key_values_count++;
139+
opal_mutex_unlock(&opal_tsd_lock);
136140
}
137141
return (ABT_SUCCESS == rc) ? OPAL_SUCCESS : OPAL_ERROR;
138142
}
@@ -141,6 +145,7 @@ int opal_tsd_keys_destruct(void)
141145
{
142146
int i;
143147
void *ptr;
148+
opal_mutex_lock(&opal_tsd_lock);
144149
for (i = 0; i < opal_tsd_key_values_count; i++) {
145150
if (OPAL_SUCCESS ==
146151
opal_tsd_getspecific(opal_tsd_key_values[i].key, &ptr)) {
@@ -152,7 +157,10 @@ int opal_tsd_keys_destruct(void)
152157
}
153158
if (0 < opal_tsd_key_values_count) {
154159
free(opal_tsd_key_values);
160+
opal_tsd_key_values = NULL;
155161
opal_tsd_key_values_count = 0;
162+
opal_tsd_key_values_size = 0;
156163
}
164+
opal_mutex_unlock(&opal_tsd_lock);
157165
return OPAL_SUCCESS;
158166
}

opal/mca/threads/pthreads/threads_pthreads_module.c

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,15 @@
3232
#include "opal/mca/threads/threads.h"
3333
#include "opal/mca/threads/tsd.h"
3434

35-
static pthread_t opal_main_thread;
36-
3735
struct opal_tsd_key_value {
3836
opal_tsd_key_t key;
3937
opal_tsd_destructor_t destructor;
4038
};
4139

40+
static opal_mutex_t opal_tsd_lock = OPAL_MUTEX_STATIC_INIT;
4241
static struct opal_tsd_key_value *opal_tsd_key_values = NULL;
4342
static int opal_tsd_key_values_count = 0;
43+
static int opal_tsd_key_values_size = 0;
4444

4545
/*
4646
* Constructor
@@ -93,13 +93,19 @@ int opal_tsd_key_create(opal_tsd_key_t *key, opal_tsd_destructor_t destructor)
9393
{
9494
int rc;
9595
rc = pthread_key_create(key, destructor);
96-
if ((0 == rc) && (pthread_self() == opal_main_thread)) {
97-
opal_tsd_key_values = (struct opal_tsd_key_value *)
98-
realloc(opal_tsd_key_values, (opal_tsd_key_values_count + 1) *
96+
if (0 == rc) {
97+
opal_mutex_lock(&opal_tsd_lock);
98+
if (opal_tsd_key_values_size <= opal_tsd_key_values_count) {
99+
opal_tsd_key_values_size = opal_tsd_key_values_size == 0
100+
? 1 : opal_tsd_key_values_size * 2;
101+
opal_tsd_key_values = (struct opal_tsd_key_value *)
102+
realloc(opal_tsd_key_values, opal_tsd_key_values_size *
99103
sizeof(struct opal_tsd_key_value));
104+
}
100105
opal_tsd_key_values[opal_tsd_key_values_count].key = *key;
101106
opal_tsd_key_values[opal_tsd_key_values_count].destructor = destructor;
102107
opal_tsd_key_values_count++;
108+
opal_mutex_unlock(&opal_tsd_lock);
103109
}
104110
return 0 == rc ? OPAL_SUCCESS : OPAL_ERR_IN_ERRNO;
105111
}
@@ -108,6 +114,7 @@ int opal_tsd_keys_destruct(void)
108114
{
109115
int i;
110116
void *ptr;
117+
opal_mutex_lock(&opal_tsd_lock);
111118
for (i = 0; i < opal_tsd_key_values_count; i++) {
112119
if (OPAL_SUCCESS ==
113120
opal_tsd_getspecific(opal_tsd_key_values[i].key, &ptr)) {
@@ -119,14 +126,16 @@ int opal_tsd_keys_destruct(void)
119126
}
120127
if (0 < opal_tsd_key_values_count) {
121128
free(opal_tsd_key_values);
129+
opal_tsd_key_values = NULL;
122130
opal_tsd_key_values_count = 0;
131+
opal_tsd_key_values_size = 0;
123132
}
133+
opal_mutex_unlock(&opal_tsd_lock);
124134
return OPAL_SUCCESS;
125135
}
126136

127137
void opal_thread_set_main(void)
128138
{
129-
opal_main_thread = pthread_self();
130139
}
131140

132141
void opal_event_use_threads(void)

0 commit comments

Comments
 (0)