Skip to content

Commit

Permalink
WT-8291 AddressSanitizer reported attempting double-free during test_…
Browse files Browse the repository at this point in the history
…wt4803_history_store_abort (#7160)

Don't call exit() without an exec, call _exit() instead.
  • Loading branch information
keithbostic committed Nov 8, 2021
1 parent ad86d0c commit c8b6e9a
Show file tree
Hide file tree
Showing 10 changed files with 65 additions and 91 deletions.
13 changes: 5 additions & 8 deletions test/csuite/random_abort/main.c
Expand Up @@ -346,7 +346,7 @@ fill_db(uint32_t nth)
*/
free(thr);
free(td);
exit(EXIT_SUCCESS);
_exit(EXIT_SUCCESS);
}

extern int __wt_optind;
Expand Down Expand Up @@ -694,12 +694,11 @@ main(int argc, char *argv[])
memset(&sa, 0, sizeof(sa));
sa.sa_handler = handler;
testutil_checksys(sigaction(SIGCHLD, &sa, NULL));
if ((pid = fork()) < 0)
testutil_die(errno, "fork");
testutil_checksys((pid = fork()) < 0);

if (pid == 0) { /* child */
fill_db(nth);
return (EXIT_SUCCESS);
/* NOTREACHED */
}

/* parent */
Expand Down Expand Up @@ -739,10 +738,8 @@ main(int argc, char *argv[])
* here.
*/
printf("Kill child\n");
if (kill(pid, SIGKILL) != 0)
testutil_die(errno, "kill");
if (waitpid(pid, &status, 0) == -1)
testutil_die(errno, "waitpid");
testutil_checksys(kill(pid, SIGKILL) != 0);
testutil_checksys(waitpid(pid, &status, 0) == -1);
}
/*
* !!! If we wanted to take a copy of the directory before recovery,
Expand Down
24 changes: 9 additions & 15 deletions test/csuite/random_directio/main.c
Expand Up @@ -622,7 +622,7 @@ fill_db(uint32_t nth, uint32_t datasize, const char *method, uint32_t flags)
*/
free(thr);
free(td);
exit(EXIT_SUCCESS);
_exit(EXIT_SUCCESS);
}

/*
Expand Down Expand Up @@ -775,12 +775,9 @@ kill_child(pid_t pid)
* abort, then signal continue so that the child process will process the abort and dump core.
*/
printf("Send abort to child process ID %d\n", (int)pid);
if (kill(pid, SIGABRT) != 0)
testutil_die(errno, "kill");
if (kill(pid, SIGCONT) != 0)
testutil_die(errno, "kill");
if (waitpid(pid, &status, 0) == -1)
testutil_die(errno, "waitpid");
testutil_checksys(kill(pid, SIGABRT) != 0);
testutil_checksys(kill(pid, SIGCONT) != 0);
testutil_checksys(waitpid(pid, &status, 0) == -1);
}

/*
Expand Down Expand Up @@ -1001,7 +998,7 @@ handler(int sig)
int status, termsig;

WT_UNUSED(sig);
pid = waitpid(-1, &status, WNOHANG | WUNTRACED);
testutil_checksys((pid = waitpid(-1, &status, WNOHANG | WUNTRACED)) == -1);
if (pid == 0)
return; /* Nothing to wait for. */
if (WIFSTOPPED(status))
Expand Down Expand Up @@ -1211,12 +1208,11 @@ main(int argc, char *argv[])
memset(&sa, 0, sizeof(sa));
sa.sa_handler = handler;
testutil_checksys(sigaction(SIGCHLD, &sa, NULL));
if ((pid = fork()) < 0)
testutil_die(errno, "fork");
testutil_checksys((pid = fork()) < 0);
}
if (pid == 0) { /* child, or populate_only */
fill_db(nth, datasize, method, flags);
return (EXIT_SUCCESS);
/* NOTREACHED */
}

/* parent */
Expand Down Expand Up @@ -1247,10 +1243,8 @@ main(int argc, char *argv[])
printf("Kill child\n");
sa.sa_handler = SIG_DFL;
testutil_checksys(sigaction(SIGCHLD, &sa, NULL));
if (kill(pid, SIGKILL) != 0)
testutil_die(errno, "kill");
if (waitpid(pid, &status, 0) == -1)
testutil_die(errno, "waitpid");
testutil_checksys(kill(pid, SIGKILL) != 0);
testutil_checksys(waitpid(pid, &status, 0) == -1);
}
if (verify_only && !check_db(nth, datasize, 0, false, flags)) {
printf("FAIL\n");
Expand Down
4 changes: 2 additions & 2 deletions test/csuite/schema_abort/main.c
Expand Up @@ -837,7 +837,7 @@ run_workload(uint32_t nth)
*/
free(thr);
free(td);
exit(EXIT_SUCCESS);
_exit(EXIT_SUCCESS);
}

extern int __wt_optind;
Expand Down Expand Up @@ -999,7 +999,7 @@ main(int argc, char *argv[])

if (pid == 0) { /* child */
run_workload(nth);
return (EXIT_SUCCESS);
/* NOTREACHED */
}

/* parent */
Expand Down
4 changes: 2 additions & 2 deletions test/csuite/tiered_abort/main.c
Expand Up @@ -509,7 +509,7 @@ run_workload(uint32_t nth, const char *build_dir)
*/
free(thr);
free(td);
exit(EXIT_SUCCESS);
_exit(EXIT_SUCCESS);
}

extern int __wt_optind;
Expand Down Expand Up @@ -674,7 +674,7 @@ main(int argc, char *argv[])

if (pid == 0) { /* child */
run_workload(nth, build_dir);
return (EXIT_SUCCESS);
/* NOTREACHED */
}

/* parent */
Expand Down
4 changes: 2 additions & 2 deletions test/csuite/timestamp_abort/main.c
Expand Up @@ -575,7 +575,7 @@ run_workload(uint32_t nth)
*/
free(thr);
free(td);
exit(EXIT_SUCCESS);
_exit(EXIT_SUCCESS);
}

extern int __wt_optind;
Expand Down Expand Up @@ -745,7 +745,7 @@ main(int argc, char *argv[])

if (pid == 0) { /* child */
run_workload(nth);
return (EXIT_SUCCESS);
/* NOTREACHED */
}

/* parent */
Expand Down
11 changes: 4 additions & 7 deletions test/csuite/truncated_log/main.c
Expand Up @@ -218,8 +218,7 @@ fill_db(void)
}
if (fclose(fp) != 0)
testutil_die(errno, "fclose");
exit(0);
/* NOTREACHED */
_exit(EXIT_SUCCESS);
}

extern int __wt_optind;
Expand Down Expand Up @@ -263,18 +262,16 @@ main(int argc, char *argv[])
/*
* Fork a child to do its work. Wait for it to exit.
*/
if ((pid = fork()) < 0)
testutil_die(errno, "fork");
testutil_checksys((pid = fork()) < 0);

if (pid == 0) { /* child */
fill_db();
return (EXIT_SUCCESS);
/* NOTREACHED */
}

/* parent */
/* Wait for child to kill itself. */
if (waitpid(pid, &status, 0) == -1)
testutil_die(errno, "waitpid");
testutil_checksys(waitpid(pid, &status, 0) == -1);

/*
* !!! If we wanted to take a copy of the directory before recovery,
Expand Down
20 changes: 9 additions & 11 deletions test/csuite/wt2909_checkpoint_integrity/main.c
Expand Up @@ -89,7 +89,7 @@ static void generate_value(uint32_t, uint64_t, char *, int *, int *, int *, char
static void run_check_subtest(TEST_OPTS *, const char *, uint64_t, bool, uint64_t *);
static int run_check_subtest_range(TEST_OPTS *, const char *, bool);
static void run_check_subtest_range_retry(TEST_OPTS *, const char *, bool);
static int run_process(TEST_OPTS *, const char *, char *[], int *);
static void run_process(TEST_OPTS *, const char *, char *[], int *);
static void subtest_main(int, char *[], bool);
static void subtest_populate(TEST_OPTS *, bool);

Expand Down Expand Up @@ -332,8 +332,7 @@ run_check_subtest(
if (opts->verbose)
printf("running a separate process with %" PRIu64 " operations until fail...\n", nops);
testutil_clean_work_dir(opts->home);
testutil_check(
run_process(opts, debugger != NULL ? debugger : opts->argv0, subtest_args, &estatus));
run_process(opts, debugger != NULL ? debugger : opts->argv0, subtest_args, &estatus);
if (opts->verbose)
printf("process exited %d\n", estatus);

Expand Down Expand Up @@ -443,8 +442,8 @@ run_check_subtest_range_retry(TEST_OPTS *opts, const char *debugger, bool close_
* run_process --
* Run a program with arguments, wait until it completes.
*/
static int
run_process(TEST_OPTS *opts, const char *prog, char *argv[], int *status)
static void
run_process(TEST_OPTS *opts, const char *prog, char *argv[], int *statusp)
{
int pid;
char **arg;
Expand All @@ -455,14 +454,13 @@ run_process(TEST_OPTS *opts, const char *prog, char *argv[], int *status)
printf("%s ", *arg);
printf("\n");
}
if ((pid = fork()) == 0) {
testutil_checksys((pid = fork()) < 0);
if (pid == 0) {
(void)execv(prog, argv);
testutil_die(errno, "%s", prog);
} else if (pid < 0)
return (errno);
_exit(EXIT_FAILURE);
}

(void)waitpid(pid, status, 0);
return (0);
testutil_checksys(waitpid(pid, statusp, 0) == -1);
}

/*
Expand Down
48 changes: 20 additions & 28 deletions test/csuite/wt4803_history_store_abort/main.c
Expand Up @@ -66,7 +66,7 @@ handle_message(WT_EVENT_HANDLER *handler, WT_SESSION *session, int error, const
* panic.
*/
if (expect_panic)
exit(EXIT_SUCCESS);
_exit(EXIT_SUCCESS);
}

return (0);
Expand All @@ -82,6 +82,12 @@ hs_workload(TEST_OPTS *opts, const char *hs_file_max)
int i;
char buf[WT_MEGABYTE], open_config[128];

/*
* We're going to run this workload for different configurations of file_max. So clean out the
* work directory each time.
*/
testutil_make_work_dir(opts->home);

testutil_check(__wt_snprintf(open_config, sizeof(open_config),
"create,cache_size=50MB,history_store=(file_max=%s)", hs_file_max));

Expand Down Expand Up @@ -125,37 +131,25 @@ hs_workload(TEST_OPTS *opts, const char *hs_file_max)
* Cleanup. We do not get here when the file_max size is small because we will have already hit
* the maximum and exited. This code only executes on the successful path.
*/
testutil_check(other_session->rollback_transaction(other_session, NULL));
testutil_check(other_session->close(other_session, NULL));

testutil_check(cursor->close(cursor));
testutil_check(session->close(session, NULL));
testutil_check(opts->conn->close(opts->conn, NULL));
}

static int
static void
test_hs_workload(TEST_OPTS *opts, const char *hs_file_max)
{
pid_t pid;
int status;

/*
* We're going to run this workload for different configurations of file_max. So clean out the
* work directory each time.
*/
testutil_make_work_dir(opts->home);

/*
* Since it's possible that the workload will panic and abort, we will fork the process and
* execute the workload in the child process.
*
* This way, we can safely check the exit code of the child process and confirm that it is what
* we expected.
*/
pid = fork();
if (pid < 0)
/* Failed fork. */
testutil_die(errno, "fork");
else if (pid == 0) {
testutil_checksys((pid = fork()) < 0);

if (pid == 0) {
/* Child process from here. */
hs_workload(opts, hs_file_max);

Expand All @@ -167,16 +161,14 @@ test_hs_workload(TEST_OPTS *opts, const char *hs_file_max)
expect_panic ? "true" : "false");

if (expect_panic)
exit(EXIT_FAILURE);
_exit(EXIT_FAILURE);
else
exit(EXIT_SUCCESS);
_exit(EXIT_SUCCESS);
}

/* Parent process from here. */
if (waitpid(pid, &status, 0) == -1)
testutil_die(errno, "waitpid");

return (status);
testutil_checksys(waitpid(pid, &status, 0) == -1);
testutil_assert(status == 0);
}

int
Expand All @@ -192,23 +184,23 @@ main(int argc, char **argv)
* needed.
*/
expect_panic = false;
testutil_check(test_hs_workload(&opts, "0"));
test_hs_workload(&opts, "0");

/*
* The history store is limited to 5GB. This is more than enough for this workload so we don't
* expect any failure.
*/
expect_panic = false;
testutil_check(test_hs_workload(&opts, "5GB"));
test_hs_workload(&opts, "5GB");

/*
* The history store is limited to 100MB. This is insufficient for this workload so we're
* expecting a failure.
*/
expect_panic = true;
testutil_check(test_hs_workload(&opts, "100MB"));
test_hs_workload(&opts, "100MB");

testutil_cleanup(&opts);

return (0);
return (EXIT_SUCCESS);
}
4 changes: 2 additions & 2 deletions test/csuite/wt6616_checkpoint_oldest_ts/main.c
Expand Up @@ -228,7 +228,7 @@ run_workload(void)

/* NOTREACHED */
free(thr);
exit(EXIT_SUCCESS);
_exit(EXIT_SUCCESS);
}

/*
Expand Down Expand Up @@ -316,7 +316,7 @@ main(int argc, char *argv[])

if (pid == 0) { /* child */
run_workload();
return (EXIT_SUCCESS);
/* NOTREACHED */
}

/* parent */
Expand Down

0 comments on commit c8b6e9a

Please sign in to comment.