Fix race condition with page cache descriptors (#7478)
Markos Fountoulakis committed
Dec 9, 2019 at 17:01 UTC
c177a4a8fc5ae37be0e28e1680d08399b3969258
2 files changed
+13
-7
database/engine/pagecache.c
+2
-2
@@ -429,8 +429,8 @@ void pg_cache_punch_hole(struct rrdengine_instance *ctx, struct rrdeng_page_desc
429
uv_rwlock_wrunlock(&pg_cache->pg_cache_rwlock);
430
}
431
pg_cache_put(ctx, descr);
432
-
433
- rrdeng_destroy_pg_cache_descr(ctx, pg_cache_descr);
432
+ if (descr->pg_cache_descr_state & PG_CACHE_DESCR_ALLOCATED)
433
+ rrdeng_try_deallocate_pg_cache_descr(ctx, descr);
434
destroy:
435
freez(descr);
436
pg_cache_update_metric_times(page_index);
database/engine/rrdenglocking.c
+11
-5
@@ -20,7 +20,12 @@ struct page_cache_descr *rrdeng_create_pg_cache_descr(struct rrdengine_instance
20
21
void rrdeng_destroy_pg_cache_descr(struct rrdengine_instance *ctx, struct page_cache_descr *pg_cache_descr)
22
{
23
+ /* Flush any lock and condition variable users */
24
+ uv_mutex_lock(&pg_cache_descr->mutex);
25
+ uv_mutex_unlock(&pg_cache_descr->mutex);
26
+
27
uv_cond_destroy(&pg_cache_descr->cond);
28
+
29
uv_mutex_destroy(&pg_cache_descr->mutex);
30
freez(pg_cache_descr);
31
rrd_stat_atomic_add(&ctx->stats.page_cache_descriptors, -1);
@@ -71,8 +76,9 @@ void rrdeng_page_descr_mutex_lock(struct rrdengine_instance *ctx, struct rrdeng_
76
continue; /* spin */
77
}
78
/* page cache descriptor is already allocated */
74
- assert(old_state & PG_CACHE_DESCR_ALLOCATED);
75
-
79
+ if (unlikely(!(old_state & PG_CACHE_DESCR_ALLOCATED))) {
80
+ fatal("Invalid page cache descriptor locking state:%#lX", old_state);
81
+ }
82
new_state = (old_users + 1) << PG_CACHE_DESCR_SHIFT;
83
new_state |= old_state & PG_CACHE_DESCR_FLAGS_MASK;
84
@@ -122,7 +128,7 @@ void rrdeng_page_descr_mutex_unlock(struct rrdengine_instance *ctx, struct rrden
128
pg_cache_descr = descr->pg_cache_descr;
129
/* caller is the only page cache descriptor user and there are no pending references on the page */
130
if ((old_state & PG_CACHE_DESCR_DESTROY) && (1 == old_users) &&
125
- !pg_cache_descr->flags && !pg_cache_descr->refcnt) {
131
+ !pg_cache_descr->flags && !pg_cache_descr->refcnt && !pg_cache_descr->waiters) {
132
new_state = PG_CACHE_DESCR_LOCKED;
133
ret_state = ulong_compare_and_swap(&descr->pg_cache_descr_state, old_state, new_state);
134
if (old_state == ret_state) {
@@ -171,7 +177,7 @@ void rrdeng_try_deallocate_pg_cache_descr(struct rrdengine_instance *ctx, struct
177
must_unlock = 1;
178
just_locked = 0;
179
/* Try deallocate if there are no pending references on the page */
174
- if (!pg_cache_descr->flags && !pg_cache_descr->refcnt) {
180
+ if (!pg_cache_descr->flags && !pg_cache_descr->refcnt && !pg_cache_descr->waiters) {
181
rrdeng_destroy_pg_cache_descr(ctx, pg_cache_descr);
182
we_freed = 1;
183
/* success */
@@ -204,13 +210,13 @@ void rrdeng_try_deallocate_pg_cache_descr(struct rrdengine_instance *ctx, struct
210
assert(0 == old_users);
211
continue; /* spin */
212
}
207
- pg_cache_descr = descr->pg_cache_descr;
213
/* caller is the only page cache descriptor user */
214
if (0 == old_users) {
215
new_state = old_state | PG_CACHE_DESCR_LOCKED;
216
ret_state = ulong_compare_and_swap(&descr->pg_cache_descr_state, old_state, new_state);
217
if (old_state == ret_state) {
218
just_locked = 1;
219
+ pg_cache_descr = descr->pg_cache_descr;
220
/* retry */
221
continue;
222
}