Skip to content

Commit

Permalink
Don't crash on destroy() when process was forked (#1674)
Browse files Browse the repository at this point in the history
  • Loading branch information
edenhill committed Feb 7, 2018
1 parent 06aecfa commit 8c67e42
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 1 deletion.
5 changes: 4 additions & 1 deletion src/rdkafka.c
Original file line number Diff line number Diff line change
Expand Up @@ -734,7 +734,10 @@ static void rd_kafka_destroy_app (rd_kafka_t *rk, int blocking) {
"Joining main background thread");

if (thrd_join(thrd, NULL) != thrd_success)
rd_kafka_assert(NULL, !*"failed to join main thread");
rd_kafka_log(rk, LOG_ERR, "DESTROY",
"Failed to join main thread: %s "
"(was process forked?)",
rd_strerror(errno));

rd_kafka_destroy_final(rk);
}
Expand Down
91 changes: 91 additions & 0 deletions tests/0079-fork.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
* librdkafka - Apache Kafka C library
*
* Copyright (c) 2012-2015, Magnus Edenhill
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* 1. Redistributions of source code must retain the above copyright notice,
* this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*/

#include "test.h"
#include "rdkafka.h"

#ifndef _MSC_VER
#include <unistd.h>
#include <sys/wait.h>
#endif

/**
* @brief Forking a threaded process will not transfer threads (such as
* librdkafka's background threads) to the child process.
* There is no way such a forked client instance will work
* in the child process, but it should not crash on destruction: #1674
*/

int main_0079_fork (int argc, char **argv) {
#ifdef _MSC_VER
TEST_SKIP("No fork() support on Windows");
return 0;
#else
pid_t pid;
rd_kafka_t *rk;
int status;

rk = test_create_producer();

rd_kafka_producev(rk,
RD_KAFKA_V_TOPIC("atopic"),
RD_KAFKA_V_VALUE("hi", 2),
RD_KAFKA_V_END);

pid = fork();
TEST_ASSERT(pid != 1, "fork() failed: %s", strerror(errno));

if (pid == 0) {
/* Child process */

/* This call will enqueue the message on a queue
* which is not served by any thread, but it should not crash */
rd_kafka_producev(rk,
RD_KAFKA_V_TOPIC("atopic"),
RD_KAFKA_V_VALUE("hello", 5),
RD_KAFKA_V_END);

/* Don't crash on us */
rd_kafka_destroy(rk);

exit(0);
}

/* Parent process, wait for child to exit cleanly. */
if (waitpid(pid, &status, 0) == -1)
TEST_FAIL("waitpid(%d) failed: %s", (int)pid, strerror(errno));

if (!WIFEXITED(status) ||
WEXITSTATUS(status) != 0)
TEST_FAIL("child exited with status %d", WEXITSTATUS(status));

rd_kafka_destroy(rk);

return 0;
#endif
}
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ set(
0076-produce_retry.c
0077-compaction.c
0078-c_from_cpp.cpp
0079-fork.c
8000-idle.cpp
test.c
testcpp.cpp
Expand Down
2 changes: 2 additions & 0 deletions tests/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ _TEST_DECL(0075_retry);
_TEST_DECL(0076_produce_retry);
_TEST_DECL(0077_compaction);
_TEST_DECL(0078_c_from_cpp);
_TEST_DECL(0079_fork);


/* Manual tests */
Expand Down Expand Up @@ -261,6 +262,7 @@ struct test tests[] = {
_TEST(0076_produce_retry, 0),
_TEST(0077_compaction, 0, TEST_BRKVER(0,9,0,0)),
_TEST(0078_c_from_cpp, TEST_F_LOCAL),
_TEST(0079_fork, TEST_F_LOCAL|TEST_F_KNOWN_ISSUE_WIN32),

/* Manual tests */
_TEST(8000_idle, TEST_F_MANUAL),
Expand Down
1 change: 1 addition & 0 deletions win32/tests/tests.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@
<ClCompile Include="..\..\tests\0076-produce_retry.c" />
<ClCompile Include="..\..\tests\0077-compaction.c" />
<ClCompile Include="..\..\tests\0078-c_from_cpp.cpp" />
<ClCompile Include="..\..\tests\0079-fork.c" />
<ClCompile Include="..\..\tests\8000-idle.cpp" />
<ClCompile Include="..\..\tests\test.c" />
<ClCompile Include="..\..\tests\testcpp.cpp" />
Expand Down

0 comments on commit 8c67e42

Please sign in to comment.