Skip to content

Commit c123d54

Browse files
committed
job-list: store jobid in idsync list struct
Problem: In the idsync API a list is stored in a hash and an externally stored jobid in a list entry is used as the key. This currently is fine because the entire list is read back and destroyed in idsync_check_waiting_id(). However, if we ever want to read/delete some list entries but not others, the externally stored jobid makes the hash lookup unsafe, as the storage of the key can no longer be guaranteed. Solution: Store the list in a new struct that also stores the jobid. Also, adjust the name of a cleanup function as a result.
1 parent 01f18eb commit c123d54

File tree

1 file changed

+32
-16
lines changed

1 file changed

+32
-16
lines changed

src/modules/job-list/idsync.c

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@
2222
#include "idsync.h"
2323
#include "job_util.h"
2424

25+
/* Used in waits hash, need to store job id within data structure for lookup */
26+
struct idsync_wait_list {
27+
zlistx_t *l;
28+
flux_jobid_t id;
29+
};
30+
2531
void idsync_data_destroy (void *data)
2632
{
2733
if (data) {
@@ -70,10 +76,15 @@ static struct idsync_data *idsync_data_create (flux_t *h,
7076
return NULL;
7177
}
7278

73-
static void idsync_waits_list_destroy (void **data)
79+
static void idsync_wait_list_destroy (void **data)
7480
{
75-
if (data)
76-
zlistx_destroy ((zlistx_t **) data);
81+
if (data) {
82+
struct idsync_wait_list *iwl = *data;
83+
if (iwl) {
84+
zlistx_destroy (&iwl->l);
85+
free (iwl);
86+
}
87+
}
7788
}
7889

7990
struct idsync_ctx *idsync_ctx_create (flux_t *h)
@@ -93,7 +104,7 @@ struct idsync_ctx *idsync_ctx_create (flux_t *h)
93104
if (!(isctx->waits = job_hash_create ()))
94105
goto error;
95106

96-
zhashx_set_destructor (isctx->waits, idsync_waits_list_destroy);
107+
zhashx_set_destructor (isctx->waits, idsync_wait_list_destroy);
97108

98109
return isctx;
99110

@@ -174,26 +185,31 @@ void idsync_check_id_valid_cleanup (struct idsync_ctx *isctx,
174185
static int idsync_add_waiter (struct idsync_ctx *isctx,
175186
struct idsync_data *isd)
176187
{
177-
zlistx_t *list_isd;
188+
struct idsync_wait_list *iwl = NULL;
178189

179190
/* isctx->waits holds lists of ids waiting on, b/c multiple callers
180191
* could wait on same id */
181-
if (!(list_isd = zhashx_lookup (isctx->waits, &isd->id))) {
182-
if (!(list_isd = zlistx_new ()))
192+
if (!(iwl = zhashx_lookup (isctx->waits, &isd->id))) {
193+
iwl = calloc (1, sizeof (*iwl));
194+
if (!iwl)
195+
goto enomem;
196+
197+
if (!(iwl->l = zlistx_new ()))
183198
goto enomem;
184-
zlistx_set_destructor (list_isd, idsync_data_destroy_wrapper);
199+
zlistx_set_destructor (iwl->l, idsync_data_destroy_wrapper);
200+
iwl->id = isd->id;
185201

186-
if (zhashx_insert (isctx->waits, &isd->id, list_isd) < 0)
202+
if (zhashx_insert (isctx->waits, &iwl->id, iwl) < 0)
187203
goto enomem;
188204
}
189205

190-
if (!zlistx_add_end (list_isd, isd))
206+
if (!zlistx_add_end (iwl->l, isd))
191207
goto enomem;
192208

193209
return 0;
194210

195211
enomem:
196-
zlistx_destroy (&list_isd);
212+
idsync_wait_list_destroy ((void **)&iwl);
197213
errno = ENOMEM;
198214
return -1;
199215
}
@@ -248,16 +264,16 @@ static void idsync_data_respond (struct idsync_ctx *isctx,
248264

249265
void idsync_check_waiting_id (struct idsync_ctx *isctx, struct job *job)
250266
{
251-
zlistx_t *list_isd;
267+
struct idsync_wait_list *iwl;
252268

253-
if ((list_isd = zhashx_lookup (isctx->waits, &job->id))) {
269+
if ((iwl = zhashx_lookup (isctx->waits, &job->id))) {
254270
struct idsync_data *isd;
255-
isd = zlistx_first (list_isd);
271+
isd = zlistx_first (iwl->l);
256272
while (isd) {
257273
idsync_data_respond (isctx, isd, job);
258-
isd = zlistx_next (list_isd);
274+
isd = zlistx_next (iwl->l);
259275
}
260-
zhashx_delete (isctx->waits, &job->id);
276+
zhashx_delete (isctx->waits, &iwl->id);
261277
}
262278
}
263279

0 commit comments

Comments
 (0)