Skip to content

Commit 08fe601

Browse files
committed
fix iterator crash with wrong keyname
1 parent 4449920 commit 08fe601

File tree

1 file changed

+40
-50
lines changed

1 file changed

+40
-50
lines changed

src/context/key_scan_cursor.rs

Lines changed: 40 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,33 @@
1-
use std::{ffi::c_void, ptr::{self, addr_of_mut}};
1+
use std::{
2+
ffi::c_void,
3+
ptr::{self, addr_of_mut},
4+
};
25

36
use crate::{key::RedisKey, raw, Context, RedisString};
47

58
/// A cursor to scan fields and values in a hash key.
6-
///
9+
///
710
/// This is a wrapper around the RedisModule_ScanKey function from the C API. It provides access via [`ScanKeyCursor::foreach] and provides
811
/// a Rust iterator.
9-
///
12+
///
1013
/// Example usage:
1114
/// ```no_run
12-
///
15+
///
1316
/// ```
14-
///
17+
///
1518
/// The iterator yields tuples of (field: RedisString, value: RedisString).
16-
///
19+
///
1720
/// ## Implementation notes
18-
///
21+
///
1922
/// The `RedisModule_ScanKey` function from the C API uses a callback to return the field and value strings. We
2023
/// distinguish two cases:
21-
///
22-
/// 1. Either the callback is called once,
23-
/// 2. or multiple times
24-
///
24+
///
25+
/// 1. Either the callback is called once,
26+
/// 2. or multiple times
27+
///
2528
/// and this depends if a rehash happens during the scan.
2629
pub struct ScanKeyCursor {
27-
key: RedisKey,
30+
key: RedisKey,
2831
inner_cursor: *mut raw::RedisModuleScanCursor,
2932
}
3033

@@ -94,14 +97,8 @@ enum IteratorState {
9497
HasBufferedItems,
9598
Done,
9699
}
97-
98-
enum StackSlotState {
99-
Empty,
100-
Filled(ScanKeyIteratorItem),
101-
}
102-
103100
struct StackSlot<'a> {
104-
state: StackSlotState,
101+
//state: StackSlotState,
105102
ctx: Context,
106103
buf: &'a mut Vec<ScanKeyIteratorItem>,
107104
}
@@ -119,12 +116,10 @@ impl ScanKeyCursorIterator<'_> {
119116

120117
fn next_scan_call(&mut self) -> Option<ScanKeyIteratorItem> {
121118
let ctx_ptr = self.cursor.key.ctx;
122-
let ctx = Context::new(ctx_ptr);
123-
119+
124120
let mut stack_slot = StackSlot {
125-
state: StackSlotState::Empty,
126121
ctx: Context::new(ctx_ptr),
127-
buf: &mut self.buf
122+
buf: &mut self.buf,
128123
};
129124

130125
let data_ptr = addr_of_mut!(stack_slot).cast::<c_void>();
@@ -149,14 +144,12 @@ impl ScanKeyCursorIterator<'_> {
149144
// we may still have buffered items
150145
}
151146

152-
let StackSlotState::Filled(reval) = stack_slot.state else {
153-
// should not happen
154-
panic!("ScanKey callback did not fill the stack slot");
155-
};
156-
157-
ctx.log_notice(&format!("next Reval field: {}, value: {}", reval.0, reval.1));
158-
159-
Some(reval)
147+
if stack_slot.buf.is_empty() {
148+
// no items were returned, try again
149+
None
150+
} else {
151+
self.next_buffered_item()
152+
}
160153
}
161154

162155
fn next_buffered_item(&mut self) -> Option<ScanKeyIteratorItem> {
@@ -166,7 +159,7 @@ impl ScanKeyCursorIterator<'_> {
166159
}
167160

168161
/// The callback that is called by `RedisModule_ScanKey` to return the field and value strings.
169-
///
162+
///
170163
/// The `data` pointer is a stack slot of type `RawData` that is used to pass the data back to the iterator.
171164
unsafe extern "C" fn foreach_callback<F: Fn(&RedisKey, &RedisString, &RedisString)>(
172165
key: *mut raw::RedisModuleKey,
@@ -182,7 +175,7 @@ unsafe extern "C" fn foreach_callback<F: Fn(&RedisKey, &RedisString, &RedisStrin
182175

183176
let callback = unsafe { &mut *(data.cast::<F>()) };
184177
callback(&key, &field, &value);
185-
178+
186179
// we're not the owner of field and value strings
187180
field.take();
188181
value.take();
@@ -191,7 +184,7 @@ unsafe extern "C" fn foreach_callback<F: Fn(&RedisKey, &RedisString, &RedisStrin
191184
}
192185

193186
/// The callback that is called by `RedisModule_ScanKey` to return the field and value strings.
194-
///
187+
///
195188
/// The `data` pointer is a stack slot of type `RawData` that is used to pass the data back to the iterator.
196189
unsafe extern "C" fn iterator_callback(
197190
_key: *mut raw::RedisModuleKey,
@@ -211,22 +204,19 @@ unsafe extern "C" fn iterator_callback(
211204
let field = RedisString::from_redis_module_string(slot.ctx.get_raw(), field);
212205
let value = RedisString::from_redis_module_string(slot.ctx.get_raw(), value);
213206

214-
match slot.state {
215-
StackSlotState::Empty => {
216-
let out = format!("CB - Fill empty slot - Field: {}, Value: {}", field, value);
217-
slot.ctx.log_notice(&out);
218-
slot.state = StackSlotState::Filled((field, value));
219-
}
220-
StackSlotState::Filled(_) => {
221-
// This is the case where the callback is called multiple times.
222-
// We need to buffer the data in the iterator state.
223-
let out = format!("CB - Buffer for future use - Field: {}, Value: {}", field, value);
224-
slot.ctx.log_notice(&out);
225-
slot.buf.push((field, value));
226-
227-
}
207+
if slot.buf.is_empty() {
208+
let out = format!("CB - Value tu return - Field: {}, Value: {}", field, value);
209+
slot.ctx.log_notice(&out);
210+
} else {
211+
// This is the case where the callback is called multiple times.
212+
// We need to buffer the data in the iterator state.
213+
let out = format!(
214+
"CB - Buffer for future use - Field: {}, Value: {}",
215+
field, value
216+
);
217+
slot.ctx.log_notice(&out);
228218
}
229-
219+
slot.buf.push((field, value));
230220
}
231221

232222
// Implements an iterator for `KeysCursor` that yields (RedisKey, *mut RedisModuleString, *mut RedisModuleString) in a Rust for loop.
@@ -244,4 +234,4 @@ impl Iterator for ScanKeyCursorIterator<'_> {
244234
IteratorState::Done => None,
245235
}
246236
}
247-
}
237+
}

0 commit comments

Comments
 (0)