Skip to content
GitLab
Menu
Projects
Groups
Snippets
Loading...
Help
Help
Support
Community forum
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
Menu
Open sidebar
ruanhaishen
redis
Commits
bf53f928
Unverified
Commit
bf53f928
authored
Jan 29, 2020
by
Salvatore Sanfilippo
Committed by
GitHub
Jan 29, 2020
Browse files
Merge pull request #6699 from guybe7/module_blocked_on_key_timeout_memleak
Modules: Fix blocked-client-related memory leak
parents
7ef2270e
d7d13721
Changes
3
Hide whitespace changes
Inline
Side-by-side
src/module.c
View file @
bf53f928
...
@@ -4277,6 +4277,15 @@ void unblockClientFromModule(client *c) {
...
@@ -4277,6 +4277,15 @@ void unblockClientFromModule(client *c) {
moduleFreeContext
(
&
ctx
);
moduleFreeContext
(
&
ctx
);
}
}
/* If we made it here and client is still blocked it means that the command
* timed-out, client was killed or disconnected and disconnect_callback was
* not implemented (or it was, but RM_UnblockClient was not called from
* within it, as it should).
* We must call moduleUnblockClient in order to free privdata and
* RedisModuleBlockedClient */
if
(
!
bc
->
unblocked
)
moduleUnblockClient
(
c
);
bc
->
client
=
NULL
;
bc
->
client
=
NULL
;
/* Reset the client for a new query since, for blocking commands implemented
/* Reset the client for a new query since, for blocking commands implemented
* into modules, we do not it immediately after the command returns (and
* into modules, we do not it immediately after the command returns (and
...
...
tests/modules/blockonkeys.c
View file @
bf53f928
...
@@ -172,13 +172,13 @@ int bpopgt_reply_callback(RedisModuleCtx *ctx, RedisModuleString **argv, int arg
...
@@ -172,13 +172,13 @@ int bpopgt_reply_callback(RedisModuleCtx *ctx, RedisModuleString **argv, int arg
REDISMODULE_NOT_USED
(
argv
);
REDISMODULE_NOT_USED
(
argv
);
REDISMODULE_NOT_USED
(
argc
);
REDISMODULE_NOT_USED
(
argc
);
RedisModuleString
*
keyname
=
RedisModule_GetBlockedClientReadyKey
(
ctx
);
RedisModuleString
*
keyname
=
RedisModule_GetBlockedClientReadyKey
(
ctx
);
long
long
gt
=
(
long
long
)
RedisModule_GetBlockedClientPrivateData
(
ctx
);
long
long
*
p
gt
=
RedisModule_GetBlockedClientPrivateData
(
ctx
);
fsl_t
*
fsl
;
fsl_t
*
fsl
;
if
(
!
get_fsl
(
ctx
,
keyname
,
REDISMODULE_READ
,
0
,
&
fsl
,
0
))
if
(
!
get_fsl
(
ctx
,
keyname
,
REDISMODULE_READ
,
0
,
&
fsl
,
0
))
return
REDISMODULE_ERR
;
return
REDISMODULE_ERR
;
if
(
!
fsl
||
fsl
->
list
[
fsl
->
length
-
1
]
<=
gt
)
if
(
!
fsl
||
fsl
->
list
[
fsl
->
length
-
1
]
<=
*
p
gt
)
return
REDISMODULE_ERR
;
return
REDISMODULE_ERR
;
RedisModule_ReplyWithLongLong
(
ctx
,
fsl
->
list
[
--
fsl
->
length
]);
RedisModule_ReplyWithLongLong
(
ctx
,
fsl
->
list
[
--
fsl
->
length
]);
...
@@ -192,10 +192,8 @@ int bpopgt_timeout_callback(RedisModuleCtx *ctx, RedisModuleString **argv, int a
...
@@ -192,10 +192,8 @@ int bpopgt_timeout_callback(RedisModuleCtx *ctx, RedisModuleString **argv, int a
}
}
void
bpopgt_free_privdata
(
RedisModuleCtx
*
ctx
,
void
*
privdata
)
{
void
bpopgt_free_privdata
(
RedisModuleCtx
*
ctx
,
void
*
privdata
)
{
/* Nothing to do because privdata is actually a 'long long',
* not a pointer to the heap */
REDISMODULE_NOT_USED
(
ctx
);
REDISMODULE_NOT_USED
(
ctx
);
R
EDISMODULE_NOT_USED
(
privdata
);
R
edisModule_Free
(
privdata
);
}
}
/* FSL.BPOPGT <key> <gt> <timeout> - Block clients until list has an element greater than <gt>.
/* FSL.BPOPGT <key> <gt> <timeout> - Block clients until list has an element greater than <gt>.
...
@@ -217,9 +215,12 @@ int fsl_bpopgt(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
...
@@ -217,9 +215,12 @@ int fsl_bpopgt(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
return
REDISMODULE_OK
;
return
REDISMODULE_OK
;
if
(
!
fsl
||
fsl
->
list
[
fsl
->
length
-
1
]
<=
gt
)
{
if
(
!
fsl
||
fsl
->
list
[
fsl
->
length
-
1
]
<=
gt
)
{
/* We use malloc so the tests in blockedonkeys.tcl can check for memory leaks */
long
long
*
pgt
=
RedisModule_Alloc
(
sizeof
(
long
long
));
*
pgt
=
gt
;
/* Key is empty or has <2 elements, we must block */
/* Key is empty or has <2 elements, we must block */
RedisModule_BlockClientOnKeys
(
ctx
,
bpopgt_reply_callback
,
bpopgt_timeout_callback
,
RedisModule_BlockClientOnKeys
(
ctx
,
bpopgt_reply_callback
,
bpopgt_timeout_callback
,
bpopgt_free_privdata
,
timeout
,
&
argv
[
1
],
1
,
(
void
*
)
gt
);
bpopgt_free_privdata
,
timeout
,
&
argv
[
1
],
1
,
p
gt
);
}
else
{
}
else
{
RedisModule_ReplyWithLongLong
(
ctx
,
fsl
->
list
[
--
fsl
->
length
]);
RedisModule_ReplyWithLongLong
(
ctx
,
fsl
->
list
[
--
fsl
->
length
]);
}
}
...
...
tests/unit/moduleapi/blockonkeys.tcl
View file @
bf53f928
...
@@ -45,18 +45,24 @@ start_server {tags {"modules"}} {
...
@@ -45,18 +45,24 @@ start_server {tags {"modules"}} {
test
{
Module client blocked on keys
(
with metadata
)
: Timeout
}
{
test
{
Module client blocked on keys
(
with metadata
)
: Timeout
}
{
r del k
r del k
set rd
[
redis_deferring_client
]
set rd
[
redis_deferring_client
]
$rd client id
set cid
[
$rd
read
]
r fsl.push k 33
r fsl.push k 33
$rd fsl.bpopgt k 35 1
$rd fsl.bpopgt k 35 1
assert_equal
{
Request timedout
}
[
$rd
read
]
assert_equal
{
Request timedout
}
[
$rd
read
]
r client kill id $cid
;
# try to smoke-out client-related memory leak
}
}
test
{
Module client blocked on keys
(
with metadata
)
: Blocked, case 1
}
{
test
{
Module client blocked on keys
(
with metadata
)
: Blocked, case 1
}
{
r del k
r del k
set rd
[
redis_deferring_client
]
set rd
[
redis_deferring_client
]
$rd client id
set cid
[
$rd
read
]
r fsl.push k 33
r fsl.push k 33
$rd fsl.bpopgt k 33 0
$rd fsl.bpopgt k 33 0
r fsl.push k 34
r fsl.push k 34
assert_equal
{
34
}
[
$rd
read
]
assert_equal
{
34
}
[
$rd
read
]
r client kill id $cid
;
# try to smoke-out client-related memory leak
}
}
test
{
Module client blocked on keys
(
with metadata
)
: Blocked, case 2
}
{
test
{
Module client blocked on keys
(
with metadata
)
: Blocked, case 2
}
{
...
@@ -70,6 +76,35 @@ start_server {tags {"modules"}} {
...
@@ -70,6 +76,35 @@ start_server {tags {"modules"}} {
assert_equal
{
36
}
[
$rd
read
]
assert_equal
{
36
}
[
$rd
read
]
}
}
test
{
Module client blocked on keys
(
with metadata
)
: Blocked, CLIENT KILL
}
{
r del k
set rd
[
redis_deferring_client
]
$rd client id
set cid
[
$rd
read
]
$rd fsl.bpopgt k 35 0
r client kill id $cid
;
# try to smoke-out client-related memory leak
}
test
{
Module client blocked on keys
(
with metadata
)
: Blocked, CLIENT UNBLOCK TIMEOUT
}
{
r del k
set rd
[
redis_deferring_client
]
$rd client id
set cid
[
$rd
read
]
$rd fsl.bpopgt k 35 0
r client unblock $cid timeout
;
# try to smoke-out client-related memory leak
assert_equal
{
Request timedout
}
[
$rd
read
]
}
test
{
Module client blocked on keys
(
with metadata
)
: Blocked, CLIENT UNBLOCK ERROR
}
{
r del k
set rd
[
redis_deferring_client
]
$rd client id
set cid
[
$rd
read
]
$rd fsl.bpopgt k 35 0
r client unblock $cid error
;
# try to smoke-out client-related memory leak
assert_error
"*unblocked*"
{
$rd
read
}
}
test
{
Module client blocked on keys does not wake up on wrong type
}
{
test
{
Module client blocked on keys does not wake up on wrong type
}
{
r del k
r del k
set rd
[
redis_deferring_client
]
set rd
[
redis_deferring_client
]
...
...
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
.
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment