Skip to content

Commit 1625d83

Browse files
committed
DISPATCH-1660: Introduced a hash_handle on the item so we can set the item->hash_handle->item to zero when the item is freed. This ensures that the hash_handle will never be able to reach the freed item. This closes #1075.
1 parent 2042f66 commit 1625d83

File tree

1 file changed

+40
-8
lines changed

1 file changed

+40
-8
lines changed

src/hash.c

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ typedef struct qd_hash_item_t {
3232
void *val;
3333
const void *val_const;
3434
} v;
35+
qd_hash_handle_t *handle;
3536
} qd_hash_item_t;
3637

3738
ALLOC_DECLARE(qd_hash_item_t);
@@ -105,11 +106,24 @@ qd_hash_t *qd_hash(int bucket_exponent, int batch_size, int value_is_const)
105106
//remove the given item from the given bucket of the given hash
106107
//return the key if non-null key pointer given, otherwise, free the memory
107108
static void qd_hash_internal_remove_item(qd_hash_t *h, bucket_t *bucket, qd_hash_item_t *item, unsigned char **key) {
108-
if (key)
109+
if (key) {
109110
*key = item->key;
110-
else
111+
}
112+
else {
111113
free(item->key);
114+
item->key = 0;
115+
}
112116
DEQ_REMOVE(bucket->items, item);
117+
118+
//
119+
// We are going to free this item, so we will set the
120+
// item pointer on the hash_handle to zero so nobody with
121+
// access to the hash_handle can ever try to get to this freed item.
122+
// The item and the hash_handle can be freed independent of one another.
123+
//
124+
if (item->handle) {
125+
item->handle->item = 0;
126+
}
113127
free_qd_hash_item_t(item);
114128
h->size--;
115129
}
@@ -151,14 +165,20 @@ static qd_hash_item_t *qd_hash_internal_insert(qd_hash_t *h, bucket_t *bucket, u
151165

152166
if (item) {
153167
*exists = 1;
154-
if (handle)
168+
if (handle) {
169+
//
170+
// If the item already exists, we return the item and also return a zero hash handle.
171+
// This means that there is ever only one hash handle for an item.
172+
//
155173
*handle = 0;
174+
}
156175
return item;
157176
}
158177

159178
item = new_qd_hash_item_t();
160179
if (!item)
161180
return 0;
181+
item->handle = 0;
162182

163183
DEQ_ITEM_INIT(item);
164184
item->key = key;
@@ -174,6 +194,15 @@ static qd_hash_item_t *qd_hash_internal_insert(qd_hash_t *h, bucket_t *bucket, u
174194
*handle = new_qd_hash_handle_t();
175195
(*handle)->bucket = bucket;
176196
(*handle)->item = item;
197+
198+
//
199+
// There is ever only one hash_handle that points to an item.
200+
// We will store that hash_handle in the item itself because
201+
// when the item is freed, the item pointer on its associated hash_handle will
202+
// be set to zero so that nobody can try to access the item via the handle after
203+
// the item is freed.
204+
//
205+
item->handle = *handle;
177206
}
178207

179208
return item;
@@ -292,7 +321,7 @@ void qd_hash_retrieve_prefix(qd_hash_t *h, qd_iterator_t *iter, void **val)
292321

293322
uint32_t hash = 0;
294323

295-
qd_hash_item_t *item;
324+
qd_hash_item_t *item = 0;
296325
while (qd_iterator_next_segment(iter, &hash)) {
297326
item = qd_hash_internal_retrieve_with_hash(h, hash, iter);
298327
if (item)
@@ -315,8 +344,7 @@ void qd_hash_retrieve_prefix_const(qd_hash_t *h, qd_iterator_t *iter, const void
315344

316345
uint32_t hash = 0;
317346

318-
qd_hash_item_t *item;
319-
347+
qd_hash_item_t *item = 0;
320348
while (qd_iterator_next_segment(iter, &hash)) {
321349
item = qd_hash_internal_retrieve_with_hash(h, hash, iter);
322350
if (item)
@@ -425,7 +453,7 @@ void qd_hash_handle_free(qd_hash_handle_t *handle)
425453

426454
const unsigned char *qd_hash_key_by_handle(const qd_hash_handle_t *handle)
427455
{
428-
if (handle)
456+
if (handle && handle->item)
429457
return handle->item->key;
430458
return 0;
431459
}
@@ -447,7 +475,11 @@ qd_error_t qd_hash_remove_by_handle(qd_hash_t *h, qd_hash_handle_t *handle)
447475

448476
qd_error_t qd_hash_remove_by_handle2(qd_hash_t *h, qd_hash_handle_t *handle, unsigned char **key)
449477
{
450-
if (!handle)
478+
//
479+
// If the handle is not supplied or if the supplied handle has no item, we don't want to proceed
480+
// removing the item by handle.
481+
//
482+
if (!handle || !handle->item)
451483
return QD_ERROR_NOT_FOUND;
452484
qd_hash_internal_remove_item(h, handle->bucket, handle->item, key);
453485
return QD_ERROR_NONE;

0 commit comments

Comments
 (0)