Skip to content

Commit 80cbd18

Browse files
committed
common BUGFIX arrays of pointers of ctx data
Makes using the ctx data safe while the array can be reallocd.
1 parent 79fb8bf commit 80cbd18

File tree

1 file changed

+88
-73
lines changed

1 file changed

+88
-73
lines changed

src/ly_common.c

Lines changed: 88 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,13 @@
4848
/**< lock for creating and destroying both private & shared context data */
4949
static pthread_rwlock_t ly_ctx_data_rwlock = PTHREAD_RWLOCK_INITIALIZER;
5050

51-
/**< sized array ([sized array](@ref sizedarrays)) of private context data.
51+
/**< sized array ([sized array](@ref sizedarrays)) of pointers to private context data (safe realloc).
5252
* The context is identified by the thread ID of the thread that created it and its address. */
53-
static struct ly_ctx_private_data *ly_private_ctx_data;
53+
static struct ly_ctx_private_data **ly_private_ctx_data;
5454

55-
/**< sized array ([sized array](@ref sizedarrays)) of shared context data.
55+
/**< sized array ([sized array](@ref sizedarrays)) of pointers to shared context data (safe realloc).
5656
* The context is identified by the memory address of the context. */
57-
static struct ly_ctx_shared_data *ly_shared_ctx_data;
57+
static struct ly_ctx_shared_data **ly_shared_ctx_data;
5858

5959
LIBYANG_API_DEF uint32_t
6060
ly_version_so_major(void)
@@ -163,18 +163,28 @@ ly_ctx_ht_pattern_free_cb(void *val_p)
163163
static void
164164
ly_ctx_private_data_remove_and_free(struct ly_ctx_private_data *private_data)
165165
{
166-
LY_ARRAY_COUNT_TYPE idx;
166+
LY_ARRAY_COUNT_TYPE u;
167167

168168
if (!private_data) {
169169
return;
170170
}
171171

172+
/* free members */
172173
ly_err_free(private_data->errs);
174+
free(private_data);
173175

174-
idx = private_data - ly_private_ctx_data;
175-
if (idx < LY_ARRAY_COUNT(ly_private_ctx_data) - 1) {
176+
/* find */
177+
LY_ARRAY_FOR(ly_private_ctx_data, u) {
178+
if (ly_private_ctx_data[u] == private_data) {
179+
break;
180+
}
181+
}
182+
assert(u < LY_ARRAY_COUNT(ly_private_ctx_data));
183+
184+
/* remove */
185+
if (u < LY_ARRAY_COUNT(ly_private_ctx_data) - 1) {
176186
/* replace the private data with the last one if it even was added */
177-
ly_private_ctx_data[idx] = ly_private_ctx_data[LY_ARRAY_COUNT(ly_private_ctx_data) - 1];
187+
ly_private_ctx_data[u] = ly_private_ctx_data[LY_ARRAY_COUNT(ly_private_ctx_data) - 1];
178188
}
179189
LY_ARRAY_DECREMENT_FREE(ly_private_ctx_data);
180190
}
@@ -190,24 +200,26 @@ static LY_ERR
190200
ly_ctx_private_data_create(const struct ly_ctx *ctx, struct ly_ctx_private_data **private_data)
191201
{
192202
pthread_t tid = pthread_self();
193-
struct ly_ctx_private_data *priv_data = NULL;
203+
struct ly_ctx_private_data **priv_data;
194204
LY_ERR rc = LY_SUCCESS;
195205

196206
*private_data = NULL;
197207

198208
/* create the private context data */
199209
LY_ARRAY_NEW_GOTO(ctx, ly_private_ctx_data, priv_data, rc, cleanup);
210+
*priv_data = calloc(1, sizeof **priv_data);
211+
LY_CHECK_ERR_GOTO(!*priv_data, LOGMEM(ctx); rc = LY_EMEM, cleanup);
200212

201213
/* fill */
202-
priv_data->tid = tid;
203-
priv_data->ctx = ctx;
204-
priv_data->errs = NULL;
214+
(*priv_data)->tid = tid;
215+
(*priv_data)->ctx = ctx;
216+
(*priv_data)->errs = NULL;
205217

206-
*private_data = priv_data;
218+
*private_data = *priv_data;
207219

208220
cleanup:
209221
if (rc) {
210-
ly_ctx_private_data_remove_and_free(priv_data);
222+
ly_ctx_private_data_remove_and_free(*priv_data);
211223
}
212224
return rc;
213225
}
@@ -216,37 +228,25 @@ ly_ctx_private_data_create(const struct ly_ctx *ctx, struct ly_ctx_private_data
216228
* @brief Get the private context data for a specific context.
217229
*
218230
* @param[in] ctx Context whose private data to get.
219-
* @param[in] lock Whether to lock the data access.
220231
* @param[in] own_data_only Whether to return only the private data of the current thread.
221232
* @return Private context data of @p ctx, NULL if not found.
222233
*/
223234
static struct ly_ctx_private_data *
224-
_ly_ctx_private_data_get(const struct ly_ctx *ctx, ly_bool lock, ly_bool own_data_only)
235+
_ly_ctx_private_data_get(const struct ly_ctx *ctx, ly_bool own_data_only)
225236
{
226-
struct ly_ctx_private_data *iter;
237+
struct ly_ctx_private_data **iter;
227238
ly_bool found = 0;
228239
pthread_t tid = pthread_self();
229240

230-
if (lock) {
231-
/* RD LOCK */
232-
pthread_rwlock_rdlock(&ly_ctx_data_rwlock);
233-
}
234-
235-
LY_ARRAY_FOR(ly_private_ctx_data, struct ly_ctx_private_data, iter) {
241+
LY_ARRAY_FOR(ly_private_ctx_data, struct ly_ctx_private_data *, iter) {
236242
/* either own - ctx and tid match, or "context's" - thread does not matter */
237-
if ((iter->ctx == ctx) && (!own_data_only || (iter->tid == tid))) {
243+
if (((*iter)->ctx == ctx) && (!own_data_only || ((*iter)->tid == tid))) {
238244
found = 1;
239245
break;
240246
}
241247
}
242248

243-
if (lock) {
244-
/* RD UNLOCK */
245-
pthread_rwlock_unlock(&ly_ctx_data_rwlock);
246-
}
247-
248-
/* pointer to the structure, cannot be changed so lock is not required */
249-
return found ? iter : NULL;
249+
return found ? *iter : NULL;
250250
}
251251

252252
struct ly_ctx_private_data *
@@ -255,8 +255,15 @@ ly_ctx_private_data_get_or_create(const struct ly_ctx *ctx)
255255
struct ly_ctx_private_data *private_data;
256256
LY_ERR r;
257257

258+
/* RD LOCK */
259+
pthread_rwlock_rdlock(&ly_ctx_data_rwlock);
260+
258261
/* try to find existing data */
259-
private_data = _ly_ctx_private_data_get(ctx, 1, 1);
262+
private_data = _ly_ctx_private_data_get(ctx, 1);
263+
264+
/* RD UNLOCK */
265+
pthread_rwlock_unlock(&ly_ctx_data_rwlock);
266+
260267
if (private_data) {
261268
return private_data;
262269
}
@@ -286,21 +293,31 @@ ly_ctx_private_data_get_or_create(const struct ly_ctx *ctx)
286293
static void
287294
ly_ctx_shared_data_remove_and_free(struct ly_ctx_shared_data *shared_data)
288295
{
289-
LY_ARRAY_COUNT_TYPE idx;
296+
LY_ARRAY_COUNT_TYPE u;
290297

291298
if (!shared_data) {
292299
return;
293300
}
294301

302+
/* free members */
295303
lyht_free(shared_data->pattern_ht, ly_ctx_ht_pattern_free_cb);
296304
lydict_clean(shared_data->data_dict);
297305
free(shared_data->data_dict);
298306
lyht_free(shared_data->leafref_links_ht, ly_ctx_ht_leafref_links_rec_free);
307+
free(shared_data);
299308

300-
idx = shared_data - ly_shared_ctx_data;
301-
if (idx < LY_ARRAY_COUNT(ly_shared_ctx_data) - 1) {
309+
/* find */
310+
LY_ARRAY_FOR(ly_shared_ctx_data, u) {
311+
if (ly_shared_ctx_data[u] == shared_data) {
312+
break;
313+
}
314+
}
315+
assert(u < LY_ARRAY_COUNT(ly_shared_ctx_data));
316+
317+
/* remove */
318+
if (u < LY_ARRAY_COUNT(ly_shared_ctx_data) - 1) {
302319
/* replace the shared data with the last one */
303-
ly_shared_ctx_data[idx] = ly_shared_ctx_data[LY_ARRAY_COUNT(ly_shared_ctx_data) - 1];
320+
ly_shared_ctx_data[u] = ly_shared_ctx_data[LY_ARRAY_COUNT(ly_shared_ctx_data) - 1];
304321
}
305322
LY_ARRAY_DECREMENT_FREE(ly_shared_ctx_data);
306323
}
@@ -316,27 +333,29 @@ static LY_ERR
316333
ly_ctx_shared_data_create(const struct ly_ctx *ctx, struct ly_ctx_shared_data **shared_data)
317334
{
318335
LY_ERR rc = LY_SUCCESS;
319-
struct ly_ctx_shared_data *shrd_data = NULL;
336+
struct ly_ctx_shared_data **shrd_data = NULL;
320337

321338
if (shared_data) {
322339
*shared_data = NULL;
323340
}
324341

325342
/* create the shared context data */
326343
LY_ARRAY_NEW_GOTO(ctx, ly_shared_ctx_data, shrd_data, rc, cleanup);
344+
*shrd_data = calloc(1, sizeof **shrd_data);
345+
LY_CHECK_ERR_GOTO(!*shrd_data, LOGMEM(ctx); rc = LY_EMEM, cleanup);
327346

328347
/* fill */
329-
shrd_data->ctx = ctx;
348+
(*shrd_data)->ctx = ctx;
330349

331350
/* pattern hash table */
332-
shrd_data->pattern_ht = lyht_new(LYHT_MIN_SIZE, sizeof(struct ly_pattern_ht_rec),
351+
(*shrd_data)->pattern_ht = lyht_new(LYHT_MIN_SIZE, sizeof(struct ly_pattern_ht_rec),
333352
ly_ctx_ht_pattern_equal_cb, NULL, 1);
334-
LY_CHECK_ERR_GOTO(!shrd_data->pattern_ht, rc = LY_EMEM, cleanup);
353+
LY_CHECK_ERR_GOTO(!(*shrd_data)->pattern_ht, rc = LY_EMEM, cleanup);
335354

336355
/* data dictionary */
337-
shrd_data->data_dict = malloc(sizeof *shrd_data->data_dict);
338-
LY_CHECK_ERR_GOTO(!shrd_data->data_dict, rc = LY_EMEM, cleanup);
339-
lydict_init(shrd_data->data_dict);
356+
(*shrd_data)->data_dict = malloc(sizeof *(*shrd_data)->data_dict);
357+
LY_CHECK_ERR_GOTO(!(*shrd_data)->data_dict, rc = LY_EMEM, cleanup);
358+
lydict_init((*shrd_data)->data_dict);
340359

341360
/* leafref set */
342361
if (ctx->opts & LY_CTX_LEAFREF_LINKING) {
@@ -345,24 +364,25 @@ ly_ctx_shared_data_create(const struct ly_ctx *ctx, struct ly_ctx_shared_data **
345364
* its memory completely during various manipulation function (e.g. remove, insert). In case of using pointers, the
346365
* pointer can be reallocated safely, while record itself remains untouched and can be accessed/modified freely
347366
* */
348-
shrd_data->leafref_links_ht = lyht_new(1, sizeof(struct lyd_leafref_links_rec *), ly_ctx_ht_leafref_links_equal_cb, NULL, 1);
349-
LY_CHECK_ERR_GOTO(!shrd_data->leafref_links_ht, rc = LY_EMEM, cleanup);
367+
(*shrd_data)->leafref_links_ht = lyht_new(1, sizeof(struct lyd_leafref_links_rec *),
368+
ly_ctx_ht_leafref_links_equal_cb, NULL, 1);
369+
LY_CHECK_ERR_GOTO(!(*shrd_data)->leafref_links_ht, rc = LY_EMEM, cleanup);
350370
}
351371

352372
/* ext clb and leafref links locks */
353-
pthread_mutex_init(&shrd_data->ext_clb_lock, NULL);
354-
pthread_mutex_init(&shrd_data->leafref_links_lock, NULL);
373+
pthread_mutex_init(&(*shrd_data)->ext_clb_lock, NULL);
374+
pthread_mutex_init(&(*shrd_data)->leafref_links_lock, NULL);
355375

356376
/* refcount */
357-
ATOMIC_STORE_RELAXED(shrd_data->refcount, 1);
377+
ATOMIC_STORE_RELAXED((*shrd_data)->refcount, 1);
358378

359379
if (shared_data) {
360-
*shared_data = shrd_data;
380+
*shared_data = *shrd_data;
361381
}
362382

363383
cleanup:
364384
if (rc) {
365-
ly_ctx_shared_data_remove_and_free(shrd_data);
385+
ly_ctx_shared_data_remove_and_free(*shrd_data);
366386
}
367387
return rc;
368388
}
@@ -371,42 +391,37 @@ ly_ctx_shared_data_create(const struct ly_ctx *ctx, struct ly_ctx_shared_data **
371391
* @brief Get the shared context data for a specific context.
372392
*
373393
* @param[in] ctx Context whose shared data to get.
374-
* @param[in] lock Whether to lock the data access.
375-
* @param[out] idx Optional index of the shared data in the array.
394+
* @return Found shared ctx data, NULL if not found.
376395
*/
377396
static struct ly_ctx_shared_data *
378-
_ly_ctx_shared_data_get(const struct ly_ctx *ctx, ly_bool lock)
397+
_ly_ctx_shared_data_get(const struct ly_ctx *ctx)
379398
{
380-
struct ly_ctx_shared_data *iter;
399+
struct ly_ctx_shared_data **iter;
381400
ly_bool found = 0;
382401

383-
if (lock) {
384-
/* RD LOCK */
385-
pthread_rwlock_rdlock(&ly_ctx_data_rwlock);
386-
}
387-
388-
LY_ARRAY_FOR(ly_shared_ctx_data, struct ly_ctx_shared_data, iter) {
389-
if (iter->ctx == ctx) {
402+
LY_ARRAY_FOR(ly_shared_ctx_data, struct ly_ctx_shared_data *, iter) {
403+
if ((*iter)->ctx == ctx) {
390404
found = 1;
391405
break;
392406
}
393407
}
394408

395-
if (lock) {
396-
/* RD UNLOCK */
397-
pthread_rwlock_unlock(&ly_ctx_data_rwlock);
398-
}
399-
400-
/* pointer to the structure, cannot be changed so lock is not required */
401-
return found ? iter : NULL;
409+
return found ? *iter : NULL;
402410
}
403411

404412
struct ly_ctx_shared_data *
405413
ly_ctx_shared_data_get(const struct ly_ctx *ctx)
406414
{
407415
struct ly_ctx_shared_data *shared_data;
408416

409-
shared_data = _ly_ctx_shared_data_get(ctx, 1);
417+
/* RD LOCK */
418+
pthread_rwlock_rdlock(&ly_ctx_data_rwlock);
419+
420+
shared_data = _ly_ctx_shared_data_get(ctx);
421+
422+
/* RD UNLOCK */
423+
pthread_rwlock_unlock(&ly_ctx_data_rwlock);
424+
410425
if (!shared_data) {
411426
/* NULL to avoid infinite loop in LOGERR */
412427
LOGERR(NULL, LY_EINT, "Context shared data not found.");
@@ -435,7 +450,7 @@ ly_ctx_data_add(const struct ly_ctx *ctx)
435450
pthread_rwlock_wrlock(&ly_ctx_data_rwlock);
436451

437452
/* check for duplicates in private context data, not allowed */
438-
private_data = _ly_ctx_private_data_get(ctx, 0, 1);
453+
private_data = _ly_ctx_private_data_get(ctx, 1);
439454
if (private_data) {
440455
/* ctx pointers can match only if the context is printed (they have the same memory address) */
441456
assert(private_data->ctx->opts & LY_CTX_INT_IMMUTABLE);
@@ -455,7 +470,7 @@ ly_ctx_data_add(const struct ly_ctx *ctx)
455470
}
456471

457472
/* check for duplicates in shared context data */
458-
shared_data = _ly_ctx_shared_data_get(ctx, 0);
473+
shared_data = _ly_ctx_shared_data_get(ctx);
459474
if (shared_data) {
460475
/* found, we can end */
461476
ATOMIC_INC_RELAXED(shared_data->refcount);
@@ -486,13 +501,13 @@ ly_ctx_data_del(const struct ly_ctx *ctx)
486501
pthread_rwlock_wrlock(&ly_ctx_data_rwlock);
487502

488503
/* free private data of all the threads for this context */
489-
while ((private_data = _ly_ctx_private_data_get(ctx, 0, 0))) {
504+
while ((private_data = _ly_ctx_private_data_get(ctx, 0))) {
490505
/* free the private data */
491506
ly_ctx_private_data_remove_and_free(private_data);
492507
}
493508

494509
/* get the shared context data */
495-
shared_data = _ly_ctx_shared_data_get(ctx, 0);
510+
shared_data = _ly_ctx_shared_data_get(ctx);
496511
if (!shared_data) {
497512
/* not found, nothing to do */
498513
goto cleanup;

0 commit comments

Comments
 (0)