Skip to content

Commit 9fe7eac

Browse files
author
Meir Shpilraien (Spielrein)
authored
Allow lock Redis from global detached context. (#350)
Allow lock Redis from global detached context. Sometimes we need to perfrom operation on Redis from a background thread, for this we need to lock Redis. We can use `ThreadSafeContext` but we might prefer not to for the following reasons: 1. Creating a `ThreadSafeContext` is costly. 2. `ThreadSafeContext` which is not attached to a client do not have the module pointer and this could cause some operations to fail. The PR adds the ability to lock Redis using the global detached context. After locking, we will get `DetachedContextGuard` object which will automatically unlock Redis when dispose. `DetachedContextGuard` implements `Deref<Context>` so it can be used just like a regular `Context` to perform operations. **Notice: This context should not be use to return any replies!!!** Future improvement is to seperate contexts for command invocation and replies so those can not be accidently misstaken, notice that this PR do not introduce any regression regarding this topic because we already have this issue with `ThreadSafeContext`.
1 parent 732b2bc commit 9fe7eac

File tree

4 files changed

+88
-9
lines changed

4 files changed

+88
-9
lines changed

examples/call.rs

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
use redis_module::{
2-
redis_module, CallOptionResp, CallOptionsBuilder, CallReply, CallResult, Context,
3-
PromiseCallReply, RedisError, RedisResult, RedisString, RedisValue, ThreadSafeContext,
2+
redis_module, BlockedClient, CallOptionResp, CallOptionsBuilder, CallReply, CallResult,
3+
Context, FutureCallReply, PromiseCallReply, RedisError, RedisResult, RedisString, RedisValue,
4+
ThreadSafeContext,
45
};
56

7+
use std::thread;
8+
69
fn call_test(ctx: &Context, _: Vec<RedisString>) -> RedisResult {
710
let res: String = ctx.call("ECHO", &["TEST"])?.try_into()?;
811
if "TEST" != &res {
@@ -110,23 +113,49 @@ fn call_test(ctx: &Context, _: Vec<RedisString>) -> RedisResult {
110113
Ok("pass".into())
111114
}
112115

113-
fn call_blocking(ctx: &Context, _: Vec<RedisString>) -> RedisResult {
116+
fn call_blocking_internal(ctx: &Context) -> PromiseCallReply {
114117
let call_options = CallOptionsBuilder::new().build_blocking();
115-
let res = ctx.call_blocking("blpop", &call_options, &["list", "1"]);
118+
ctx.call_blocking("blpop", &call_options, &["list", "1"])
119+
}
120+
121+
fn call_blocking_handle_future(ctx: &Context, f: FutureCallReply, blocked_client: BlockedClient) {
122+
let future_handler = f.set_unblock_handler(move |_ctx, reply| {
123+
let thread_ctx = ThreadSafeContext::with_blocked_client(blocked_client);
124+
thread_ctx.reply(reply.map_or_else(|e| Err(e.into()), |v| Ok((&v).into())));
125+
});
126+
future_handler.dispose(ctx);
127+
}
128+
129+
fn call_blocking(ctx: &Context, _: Vec<RedisString>) -> RedisResult {
130+
let res = call_blocking_internal(ctx);
116131
match res {
117132
PromiseCallReply::Resolved(r) => r.map_or_else(|e| Err(e.into()), |v| Ok((&v).into())),
118133
PromiseCallReply::Future(f) => {
119134
let blocked_client = ctx.block_client();
120-
let future_handler = f.set_unblock_handler(move |_ctx, reply| {
121-
let thread_ctx = ThreadSafeContext::with_blocked_client(blocked_client);
122-
thread_ctx.reply(reply.map_or_else(|e| Err(e.into()), |v| Ok((&v).into())));
123-
});
124-
future_handler.dispose(ctx);
135+
call_blocking_handle_future(ctx, f, blocked_client);
125136
Ok(RedisValue::NoReply)
126137
}
127138
}
128139
}
129140

141+
fn call_blocking_from_detach_ctx(ctx: &Context, _: Vec<RedisString>) -> RedisResult {
142+
let blocked_client = ctx.block_client();
143+
thread::spawn(move || {
144+
let ctx_guard = redis_module::MODULE_CONTEXT.lock();
145+
let res = call_blocking_internal(&ctx_guard);
146+
match res {
147+
PromiseCallReply::Resolved(r) => {
148+
let thread_ctx = ThreadSafeContext::with_blocked_client(blocked_client);
149+
thread_ctx.reply(r.map_or_else(|e| Err(e.into()), |v| Ok((&v).into())));
150+
}
151+
PromiseCallReply::Future(f) => {
152+
call_blocking_handle_future(&ctx_guard, f, blocked_client);
153+
}
154+
}
155+
});
156+
Ok(RedisValue::NoReply)
157+
}
158+
130159
//////////////////////////////////////////////////////
131160

132161
redis_module! {
@@ -137,5 +166,6 @@ redis_module! {
137166
commands: [
138167
["call.test", call_test, "", 0, 0, 0],
139168
["call.blocking", call_blocking, "", 0, 0, 0],
169+
["call.blocking_from_detached_ctx", call_blocking_from_detach_ctx, "", 0, 0, 0],
140170
],
141171
}

src/context/mod.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use crate::raw::{ModuleOptions, Version};
1313
use crate::redisvalue::RedisValueKey;
1414
use crate::{add_info_field_long_long, add_info_field_str, raw, utils, Status};
1515
use crate::{RedisError, RedisResult, RedisString, RedisValue};
16+
use std::ops::Deref;
1617

1718
use std::ffi::CStr;
1819

@@ -159,6 +160,34 @@ impl Default for DetachedContext {
159160
}
160161
}
161162

163+
/// This object is returned after locking Redis from [DetachedContext].
164+
/// On dispose, Redis will be unlocked.
165+
/// This object implements [Deref] for [Context] so it can be used
166+
/// just like any Redis [Context] for command invocation.
167+
/// **This object should not be used to return replies** because there is
168+
/// no real client behind this context to return replies to.
169+
pub struct DetachedContextGuard {
170+
pub(crate) ctx: Context,
171+
}
172+
173+
unsafe impl RedisLockIndicator for DetachedContextGuard {}
174+
175+
impl Drop for DetachedContextGuard {
176+
fn drop(&mut self) {
177+
unsafe {
178+
raw::RedisModule_ThreadSafeContextUnlock.unwrap()(self.ctx.ctx);
179+
};
180+
}
181+
}
182+
183+
impl Deref for DetachedContextGuard {
184+
type Target = Context;
185+
186+
fn deref(&self) -> &Self::Target {
187+
&self.ctx
188+
}
189+
}
190+
162191
impl DetachedContext {
163192
pub fn log(&self, level: RedisLogLevel, message: &str) {
164193
let c = self.ctx.load(Ordering::Relaxed);
@@ -190,6 +219,17 @@ impl DetachedContext {
190219
self.ctx.store(ctx, Ordering::Relaxed);
191220
Ok(())
192221
}
222+
223+
/// Lock Redis for command invocation. Returns [DetachedContextGuard] which will unlock Redis when dispose.
224+
/// [DetachedContextGuard] implements [Deref<Target = Context>] so it can be used just like any Redis [Context] for command invocation.
225+
/// Locking Redis when Redis is already locked by the current thread is left unspecified.
226+
/// However, this function will not return on the second call (it might panic or deadlock, for example)..
227+
pub fn lock(&self) -> DetachedContextGuard {
228+
let c = self.ctx.load(Ordering::Relaxed);
229+
unsafe { raw::RedisModule_ThreadSafeContextLock.unwrap()(c) };
230+
let ctx = Context::new(c);
231+
DetachedContextGuard { ctx }
232+
}
193233
}
194234

195235
unsafe impl Send for DetachedContext {}

src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,21 @@ pub use crate::raw::NotifyEvent;
2727

2828
pub use crate::configuration::ConfigurationValue;
2929
pub use crate::configuration::EnumConfigurationValue;
30+
pub use crate::context::call_reply::FutureCallReply;
3031
pub use crate::context::call_reply::{CallReply, CallResult, ErrorReply, PromiseCallReply};
3132
pub use crate::context::commands;
3233
pub use crate::context::keys_cursor::KeysCursor;
3334
pub use crate::context::server_events;
3435
pub use crate::context::AclPermissions;
36+
#[cfg(feature = "min-redis-compatibility-version-7-2")]
3537
pub use crate::context::BlockingCallOptions;
3638
pub use crate::context::CallOptionResp;
3739
pub use crate::context::CallOptions;
3840
pub use crate::context::CallOptionsBuilder;
3941
pub use crate::context::Context;
4042
pub use crate::context::ContextFlags;
4143
pub use crate::context::DetachedContext;
44+
pub use crate::context::DetachedContextGuard;
4245
pub use crate::raw::*;
4346
pub use crate::redismodule::*;
4447
use backtrace::Backtrace;

tests/integration.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -600,5 +600,11 @@ fn test_call_blocking() -> Result<()> {
600600

601601
assert_eq!(res, None);
602602

603+
let res: Option<String> = redis::cmd("call.blocking_from_detached_ctx")
604+
.query(&mut con)
605+
.with_context(|| "failed to run string.set")?;
606+
607+
assert_eq!(res, None);
608+
603609
Ok(())
604610
}

0 commit comments

Comments
 (0)