diff --git a/Makefile.am b/Makefile.am index e12d51a0..3c3bddde 100644 --- a/Makefile.am +++ b/Makefile.am @@ -521,6 +521,7 @@ TESTS += tests/test-zmq-endpoints.sh TESTS += tests/test-socket-frag.sh TESTS += tests/test-msgmod.sh TESTS += tests/test-kicker.sh +TESTS += tests/test-kicker-concurrent.sh TESTS += tests/test-daemon.sh TESTS += tests/test-sample.sh diff --git a/nmsg/io.c b/nmsg/io.c index 9f13fb06..d8123b69 100644 --- a/nmsg/io.c +++ b/nmsg/io.c @@ -741,30 +741,24 @@ io_write(struct nmsg_io_thr *iothr, struct nmsg_io_output *io_output, nmsg_io_t io = iothr->io; nmsg_res res; - /* It's possible a set "count" has been reached. */ - check_close_event(iothr, io_output, 1); - - if (io->stop) { - reset_close_event(iothr, io_output); - nmsg_message_destroy(&msg); - return (nmsg_res_stop); - } res = nmsg_output_write(io_output->output, msg); if (io_output->output->type != nmsg_output_type_callback) nmsg_message_destroy(&msg); - /* - * Reset only after the write, in case another thread invokes - * check_close_event and makes changes to io_output in the meantime. - */ - reset_close_event(iothr, io_output); - if (res != nmsg_res_success) return (res); atomic_fetch_add_explicit(&io->io_count_nmsg_payload_out, 1, memory_order_relaxed); + /* It's possible a set "count" has been reached. */ + check_close_event(iothr, io_output, 1); + reset_close_event(iothr, io_output); + + if (io->stop) { + return (nmsg_res_stop); + } + return (res); } @@ -797,6 +791,8 @@ check_close_event(struct nmsg_io_thr *iothr, struct nmsg_io_output *io_output, u if (io->count > 0 && io_output->count_next_close == 0) io_output->count_next_close = io->count; + io_output->count_nmsg_payload_out += count; + if (io->count > 0 && io_output->count_nmsg_payload_out == io_output->count_next_close) { @@ -856,13 +852,6 @@ check_close_event(struct nmsg_io_thr *iothr, struct nmsg_io_output *io_output, u } out: - /* - * This incr is implicitly locked IF it's used, and this counter is - * only used IF io->count > 0; that condition results in an acquired - * lock at the beginning of this function. - */ - io_output->count_nmsg_payload_out += count; - if (io->close_fp != NULL || io->count > 0) pthread_mutex_unlock(&io_output->lock); } diff --git a/tests/test-kicker-concurrent.sh b/tests/test-kicker-concurrent.sh new file mode 100644 index 00000000..cf688416 --- /dev/null +++ b/tests/test-kicker-concurrent.sh @@ -0,0 +1,22 @@ +#!/bin/sh + +nmsgtool=$abs_top_builddir/src/nmsgtool +sample=$abs_top_srcdir/tests/generic-tests/lorem.json +outdir=$abs_top_builddir/tests/test-kicker-concurrent/ +mkdir -p "$outdir"; cd "$outdir" + +for i in 1 2 3 4 5 6 7 8; do cp "$sample" "in$i.json"; done + +TSAN_OPTIONS="halt_on_error=1 exitcode=99" \ +"$nmsgtool" -ddd \ + -j in1.json -j in2.json -j in3.json -j in4.json \ + -j in5.json -j in6.json -j in7.json -j in8.json \ + -c 1 -k "echo" -w out > kicked.list 2>run.log +rc=$? + +[ $rc -ne 99 ] || { echo "FAIL: race condition "; cat run.log; exit 1; } +[ $rc -eq 0 ] || { echo "FAIL: nmsgtool exit $rc"; cat run.log; exit 1; } + +files=$(wc -l < kicked.list) +[ "$files" -ge 100 ] || { echo "FAIL: only $files rotations"; exit 1; } +echo PASS