Fix race in module fork kill test (#10717)

The purpose of the test is to kill the child while it is running.
From the last two lines we can see the child exits before being killed.
```
- Module fork started pid: 56998
* <fork> fork child started
- Killing running module fork child: 56998
* <fork> fork child exiting
signal-handler (1652267501) Received SIGUSR1 in child, exiting now.
```

In this commit, we pass an argument to `fork.create` indicating how
long it should sleep. For the fork kill test, we use a longer time to
avoid the child exiting before being killed.

Other changes:
use wait_for_condition instead of hardcoded `after 250`.
Unify the test for failing fork with the one for killing it (save time)
This commit is contained in:
Binbin 2022-05-13 01:10:38 +08:00 committed by GitHub
parent b16d1c2713
commit 586a16ad79
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 23 additions and 16 deletions

View File

@ -23,7 +23,8 @@ void done_handler(int exitcode, int bysignal, void *user_data) {
int fork_create(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) int fork_create(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
{ {
long long code_to_exit_with; long long code_to_exit_with;
if (argc != 2) { long long usleep_us;
if (argc != 3) {
RedisModule_WrongArity(ctx); RedisModule_WrongArity(ctx);
return REDISMODULE_OK; return REDISMODULE_OK;
} }
@ -34,20 +35,22 @@ int fork_create(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
} }
RedisModule_StringToLongLong(argv[1], &code_to_exit_with); RedisModule_StringToLongLong(argv[1], &code_to_exit_with);
RedisModule_StringToLongLong(argv[2], &usleep_us);
exitted_with_code = -1; exitted_with_code = -1;
child_pid = RedisModule_Fork(done_handler, (void*)0xdeadbeef); int fork_child_pid = RedisModule_Fork(done_handler, (void*)0xdeadbeef);
if (child_pid < 0) { if (fork_child_pid < 0) {
RedisModule_ReplyWithError(ctx, "Fork failed"); RedisModule_ReplyWithError(ctx, "Fork failed");
return REDISMODULE_OK; return REDISMODULE_OK;
} else if (child_pid > 0) { } else if (fork_child_pid > 0) {
/* parent */ /* parent */
child_pid = fork_child_pid;
RedisModule_ReplyWithLongLong(ctx, child_pid); RedisModule_ReplyWithLongLong(ctx, child_pid);
return REDISMODULE_OK; return REDISMODULE_OK;
} }
/* child */ /* child */
RedisModule_Log(ctx, "notice", "fork child started"); RedisModule_Log(ctx, "notice", "fork child started");
usleep(500000); usleep(usleep_us);
RedisModule_Log(ctx, "notice", "fork child exiting"); RedisModule_Log(ctx, "notice", "fork child exiting");
RedisModule_ExitFromChild(code_to_exit_with); RedisModule_ExitFromChild(code_to_exit_with);
/* unreachable */ /* unreachable */

View File

@ -13,7 +13,8 @@ start_server {tags {"modules"}} {
test {Module fork} { test {Module fork} {
# the argument to fork.create is the exitcode on termination # the argument to fork.create is the exitcode on termination
r fork.create 3 # the second argument to fork.create is passed to usleep
r fork.create 3 100000 ;# 100ms
wait_for_condition 20 100 { wait_for_condition 20 100 {
[r fork.exitcode] != -1 [r fork.exitcode] != -1
} else { } else {
@ -23,22 +24,25 @@ start_server {tags {"modules"}} {
} {3} } {3}
test {Module fork kill} { test {Module fork kill} {
r fork.create 3 # use a longer time to avoid the child exiting before being killed
after 250 r fork.create 3 100000000 ;# 100s
wait_for_condition 20 100 {
[count_log_message "fork child started"] == 2
} else {
fail "fork didn't start"
}
# module fork twice
assert_error {Fork failed} {r fork.create 0 1}
assert {[count_log_message "Can't fork for module: File exists"] eq "1"}
r fork.kill r fork.kill
assert {[count_log_message "fork child started"] eq "2"}
assert {[count_log_message "Received SIGUSR1 in child"] eq "1"} assert {[count_log_message "Received SIGUSR1 in child"] eq "1"}
# check that it wasn't printed again (the print belong to the previous test)
assert {[count_log_message "fork child exiting"] eq "1"} assert {[count_log_message "fork child exiting"] eq "1"}
} }
test {Module fork twice} {
r fork.create 0
after 250
catch {r fork.create 0}
assert {[count_log_message "Can't fork for module: File exists"] eq "1"}
}
test "Unload the module - fork" { test "Unload the module - fork" {
assert_equal {OK} [r module unload fork] assert_equal {OK} [r module unload fork]
} }