Skip to content

Commit

Permalink
fixes #1702 segfault canceling req receive while sending
Browse files Browse the repository at this point in the history
  • Loading branch information
gdamore committed Nov 26, 2023
1 parent c2b3b33 commit fe69207
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 0 deletions.
18 changes: 18 additions & 0 deletions src/sp/protocol/reqrep0/req.c
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,23 @@ req0_ctx_cancel_recv(nni_aio *aio, void *arg, int rv)
req0_sock *s = ctx->sock;

nni_mtx_lock(&s->mtx);

// So it turns out that some users start receiving before waiting
// for the send notification. In this case if receiving is
// canceled before sending completes, we need to restore the
// message for the user. It's probably a mis-design if the user
// is trying to receive without waiting for sending to complete, but
// it was reported in the field. Users who want to avoid this mess
// should just start receiving from the send completion callback.
if (ctx->send_aio != NULL) {
nni_aio_set_msg(ctx->send_aio, ctx->req_msg);
nni_msg_header_clear(ctx->req_msg);
ctx->req_msg = NULL;
nni_aio_finish_error(ctx->send_aio, NNG_ECANCELED);
ctx->send_aio = NULL;
nni_list_remove(&s->send_queue, ctx);
}

if (ctx->recv_aio == aio) {
ctx->recv_aio = NULL;

Expand All @@ -551,6 +568,7 @@ req0_ctx_cancel_recv(nni_aio *aio, void *arg, int rv)

nni_aio_finish_error(aio, rv);
}

nni_mtx_unlock(&s->mtx);
}

Expand Down
39 changes: 39 additions & 0 deletions src/sp/protocol/reqrep0/req_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,44 @@ test_req_ctx_send_twice(void)
}
}

// this tests for the case where sending and receiving are started in parallel,
// and then the receiving aio is canceled. The sending should be errored,
// and we should get back the original message. This test covers bug #1702.
static void
test_req_ctx_send_recv_abort(void)
{
nng_socket req;
nng_ctx ctx;
nng_aio * aio[2];
nng_msg * msg;

NUTS_PASS(nng_req0_open(&req));
NUTS_PASS(nng_socket_set_ms(req, NNG_OPT_RECVTIMEO, 100));

NUTS_PASS(nng_ctx_open(&ctx, req));
NUTS_PASS(nng_aio_alloc(&aio[0], NULL, NULL));
NUTS_PASS(nng_aio_alloc(&aio[1], NULL, NULL));
NUTS_PASS(nng_msg_alloc(&msg, 0));

nng_aio_set_msg(aio[0], msg);
nng_ctx_send(ctx, aio[0]);
nng_msleep(1); // just to make sure the socket has it.
nng_ctx_recv(ctx, aio[1]);
msg = NULL;
nng_aio_wait(aio[0]);
nng_aio_wait(aio[1]);
msg = nng_aio_get_msg(aio[0]); // sent message
NUTS_TRUE(msg != NULL);
NUTS_TRUE(nng_msg_len(msg) == 0);
NUTS_FAIL(nng_aio_result(aio[0]), NNG_ECANCELED);
NUTS_FAIL(nng_aio_result(aio[1]), NNG_ETIMEDOUT);
NUTS_TRUE(nng_aio_get_msg(aio[0]) != NULL);
nng_msg_free(msg);
nng_aio_free(aio[0]);
nng_aio_free(aio[1]);
NUTS_CLOSE(req);
}

static void
test_req_ctx_recv_nonblock(void)
{
Expand Down Expand Up @@ -959,6 +997,7 @@ NUTS_TESTS = {
{ "req context send close", test_req_ctx_send_close },
{ "req context send abort", test_req_ctx_send_abort },
{ "req context send twice", test_req_ctx_send_twice },
{ "req context send recv abort", test_req_ctx_send_recv_abort },
{ "req context does not poll", test_req_ctx_no_poll },
{ "req context recv close socket", test_req_ctx_recv_close_socket },
{ "req context recv nonblock", test_req_ctx_recv_nonblock },
Expand Down

0 comments on commit fe69207

Please sign in to comment.