Skip to content

Commit

Permalink
char-socket: initialize reconnect timer only when the timer doesn't s…
Browse files Browse the repository at this point in the history 
…tart

When the disconnect event is triggered in the connecting stage,
the tcp_chr_disconnect_locked may be called twice.

The first call:
    #0  qemu_chr_socket_restart_timer (chr=0x55555582ee90) at chardev/char-socket.c:120
    #1  0x000055555558e38c in tcp_chr_disconnect_locked (chr=<optimized out>) at chardev/char-socket.c:490
    #2  0x000055555558e3cd in tcp_chr_disconnect (chr=0x55555582ee90) at chardev/char-socket.c:497
    #3  0x000055555558ea32 in tcp_chr_new_client (chr=chr@entry=0x55555582ee90, sioc=sioc@entry=0x55555582f0b0) at chardev/char-socket.c:892
    #4  0x000055555558eeb8 in qemu_chr_socket_connected (task=0x55555582f300, opaque=<optimized out>) at chardev/char-socket.c:1090
    #5  0x0000555555574352 in qio_task_complete (task=task@entry=0x55555582f300) at io/task.c:196
    #6  0x00005555555745f4 in qio_task_thread_result (opaque=0x55555582f300) at io/task.c:111
    #7  qio_task_wait_thread (task=0x55555582f300) at io/task.c:190
    #8  0x000055555558f17e in tcp_chr_wait_connected (chr=0x55555582ee90, errp=0x555555802a08 <error_abort>) at chardev/char-socket.c:1013
    #9  0x0000555555567cbd in char_socket_client_reconnect_test (opaque=0x5555557fe020 <client8unix>) at tests/test-char.c:1152
The second call:
    #0  0x00007ffff5ac3277 in raise () from /lib64/libc.so.6
    #1  0x00007ffff5ac4968 in abort () from /lib64/libc.so.6
    #2  0x00007ffff5abc096 in __assert_fail_base () from /lib64/libc.so.6
    #3  0x00007ffff5abc142 in __assert_fail () from /lib64/libc.so.6
    #4  0x000055555558d10a in qemu_chr_socket_restart_timer (chr=0x55555582ee90) at chardev/char-socket.c:125
    #5  0x000055555558df0c in tcp_chr_disconnect_locked (chr=<optimized out>) at chardev/char-socket.c:490
    #6  0x000055555558df4d in tcp_chr_disconnect (chr=0x55555582ee90) at chardev/char-socket.c:497
    #7  0x000055555558e5b2 in tcp_chr_new_client (chr=chr@entry=0x55555582ee90, sioc=sioc@entry=0x55555582f0b0) at chardev/char-socket.c:892
    #8  0x000055555558e93a in tcp_chr_connect_client_sync (chr=chr@entry=0x55555582ee90, errp=errp@entry=0x7fffffffd178) at chardev/char-socket.c:944
    #9  0x000055555558ec78 in tcp_chr_wait_connected (chr=0x55555582ee90, errp=0x555555802a08 <error_abort>) at chardev/char-socket.c:1035
    #10 0x000055555556804b in char_socket_client_test (opaque=0x5555557fe020 <client8unix>) at tests/test-char.c:1023

Run test/test-char to reproduce this issue.

test-char: chardev/char-socket.c:125: qemu_chr_socket_restart_timer: Assertion `!s->reconnect_timer' failed.

Signed-off-by: Li Feng <fengli@smartx.com>
Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20200522025554.41063-1-fengli@smartx.com>
  • Loading branch information
@elmarco
Li Feng authored and elmarco committed Jul 13, 2020
1 parent d344983 commit 2b61bb7
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 18 deletions.
2 changes: 1 addition & 1 deletion chardev/char-socket.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ static void tcp_chr_disconnect_locked(Chardev *chr)
if (emit_close) {
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
if (s->reconnect_time) {
if (s->reconnect_time && !s->reconnect_timer) {
qemu_chr_socket_restart_timer(chr);
}
}
Expand Down
73 changes: 56 additions & 17 deletions tests/test-char.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -625,12 +625,14 @@ static void char_udp_test(void)
typedef struct {
int event;
bool got_pong;
CharBackend *be;
} CharSocketTestData;


#define SOCKET_PING "Hello"
#define SOCKET_PONG "World"

typedef void (*char_socket_cb)(void *opaque, QEMUChrEvent event);

static void
char_socket_event(void *opaque, QEMUChrEvent event)
Expand All @@ -639,6 +641,27 @@ char_socket_event(void *opaque, QEMUChrEvent event)
data->event = event;
}

static void
char_socket_event_with_error(void *opaque, QEMUChrEvent event)
{
static bool first_error;
CharSocketTestData *data = opaque;
CharBackend *be = data->be;
data->event = event;
switch (event) {
case CHR_EVENT_OPENED:
if (!first_error) {
first_error = true;
qemu_chr_fe_disconnect(be);
}
return;
case CHR_EVENT_CLOSED:
return;
default:
return;
}
}


static void
char_socket_read(void *opaque, const uint8_t *buf, int size)
Expand Down Expand Up @@ -699,19 +722,24 @@ char_socket_addr_to_opt_str(SocketAddress *addr, bool fd_pass,
}


static void
char_socket_ping_pong(QIOChannel *ioc)
static int
char_socket_ping_pong(QIOChannel *ioc, Error **errp)
{
char greeting[sizeof(SOCKET_PING)];
const char *response = SOCKET_PONG;

qio_channel_read_all(ioc, greeting, sizeof(greeting), &error_abort);
int ret;
ret = qio_channel_read_all(ioc, greeting, sizeof(greeting), errp);
if (ret != 0) {
object_unref(OBJECT(ioc));
return -1;
}

g_assert(memcmp(greeting, SOCKET_PING, sizeof(greeting)) == 0);

qio_channel_write_all(ioc, response, sizeof(SOCKET_PONG), &error_abort);

qio_channel_write_all(ioc, response, sizeof(SOCKET_PONG), errp);
object_unref(OBJECT(ioc));
return 0;
}


Expand All @@ -723,7 +751,7 @@ char_socket_server_client_thread(gpointer data)

qio_channel_socket_connect_sync(ioc, addr, &error_abort);

char_socket_ping_pong(QIO_CHANNEL(ioc));
char_socket_ping_pong(QIO_CHANNEL(ioc), &error_abort);

return NULL;
}
Expand Down Expand Up @@ -783,6 +811,7 @@ static void char_socket_server_test(gconstpointer opaque)

reconnect:
data.event = -1;
data.be = &be;
qemu_chr_fe_set_handlers(&be, NULL, NULL,
char_socket_event, NULL,
&data, NULL, true);
Expand Down Expand Up @@ -855,10 +884,13 @@ char_socket_client_server_thread(gpointer data)
QIOChannelSocket *ioc = data;
QIOChannelSocket *cioc;

retry:
cioc = qio_channel_socket_accept(ioc, &error_abort);
g_assert_nonnull(cioc);

char_socket_ping_pong(QIO_CHANNEL(cioc));
if (char_socket_ping_pong(QIO_CHANNEL(cioc), NULL) != 0) {
goto retry;
}

return NULL;
}
Expand All @@ -869,12 +901,13 @@ typedef struct {
const char *reconnect;
bool wait_connected;
bool fd_pass;
char_socket_cb event_cb;
} CharSocketClientTestConfig;


static void char_socket_client_test(gconstpointer opaque)
{
const CharSocketClientTestConfig *config = opaque;
const char_socket_cb event_cb = config->event_cb;
QIOChannelSocket *ioc;
char *optstr;
Chardev *chr;
Expand Down Expand Up @@ -938,8 +971,9 @@ static void char_socket_client_test(gconstpointer opaque)

reconnect:
data.event = -1;
data.be = &be;
qemu_chr_fe_set_handlers(&be, NULL, NULL,
char_socket_event, NULL,
event_cb, NULL,
&data, NULL, true);
if (config->reconnect) {
g_assert(data.event == -1);
Expand Down Expand Up @@ -977,7 +1011,7 @@ static void char_socket_client_test(gconstpointer opaque)
/* Setup a callback to receive the reply to our greeting */
qemu_chr_fe_set_handlers(&be, char_socket_can_read,
char_socket_read,
char_socket_event, NULL,
event_cb, NULL,
&data, NULL, true);
g_assert(data.event == CHR_EVENT_OPENED);
data.event = -1;
Expand Down Expand Up @@ -1422,17 +1456,20 @@ int main(int argc, char **argv)

#define SOCKET_CLIENT_TEST(name, addr) \
static CharSocketClientTestConfig client1 ## name = \
{ addr, NULL, false, false }; \
{ addr, NULL, false, false, char_socket_event}; \
static CharSocketClientTestConfig client2 ## name = \
{ addr, NULL, true, false }; \
{ addr, NULL, true, false, char_socket_event }; \
static CharSocketClientTestConfig client3 ## name = \
{ addr, ",reconnect=1", false }; \
{ addr, ",reconnect=1", false, false, char_socket_event }; \
static CharSocketClientTestConfig client4 ## name = \
{ addr, ",reconnect=1", true }; \
{ addr, ",reconnect=1", true, false, char_socket_event }; \
static CharSocketClientTestConfig client5 ## name = \
{ addr, NULL, false, true }; \
{ addr, NULL, false, true, char_socket_event }; \
static CharSocketClientTestConfig client6 ## name = \
{ addr, NULL, true, true }; \
{ addr, NULL, true, true, char_socket_event }; \
static CharSocketClientTestConfig client7 ## name = \
{ addr, ",reconnect=1", true, false, \
char_socket_event_with_error }; \
g_test_add_data_func("/char/socket/client/mainloop/" # name, \
&client1 ##name, char_socket_client_test); \
g_test_add_data_func("/char/socket/client/wait-conn/" # name, \
Expand All @@ -1444,7 +1481,9 @@ int main(int argc, char **argv)
g_test_add_data_func("/char/socket/client/mainloop-fdpass/" # name, \
&client5 ##name, char_socket_client_test); \
g_test_add_data_func("/char/socket/client/wait-conn-fdpass/" # name, \
&client6 ##name, char_socket_client_test)
&client6 ##name, char_socket_client_test); \
g_test_add_data_func("/char/socket/client/reconnect-error/" # name, \
&client7 ##name, char_socket_client_test)

if (has_ipv4) {
SOCKET_SERVER_TEST(tcp, &tcpaddr);
Expand Down

0 comments on commit 2b61bb7

Please sign in to comment.