From a08bcfdbd34ccbfa9b9aa967c91ad370c5953da2 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 14 Feb 2018 16:48:29 +1030 Subject: [PATCH] jsonrpc: don't crash on multiple commands at once. Once we read a command, we are supposed to io_wait until it finishes. However, we are actually woken in two places: when it's complete (which is correct), and when it's written out (which is wrong). We don't care when it's written out, only when it's finished: refactor to make json_done() free and NULL the old ->current, rather than have the callers do it. Now it's clear that it's ready for both new output and new input. Fixes: #934 Signed-off-by: Rusty Russell --- lightningd/jsonrpc.c | 12 +++++++----- tests/test_lightningd.py | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index 2990d1c44..5d98546d1 100644 --- a/lightningd/jsonrpc.c +++ b/lightningd/jsonrpc.c @@ -242,8 +242,13 @@ static void json_done(struct json_connection *jcon, const char *json TAKES) struct json_output *out = tal(jcon, struct json_output); out->json = tal_strdup(out, json); - /* Queue for writing, and wake writer (and maybe reader). */ + /* Clear existing command (if any: NULL in malformed case) */ + jcon->current = tal_free(jcon->current); + + /* Queue for writing. */ list_add_tail(&jcon->output, &out->list); + + /* Both writer and reader can now continue. */ io_wake(jcon); } @@ -378,7 +383,6 @@ void command_success(struct command *cmd, struct json_result *result) assert(jcon->current == cmd); connection_complete_ok(jcon, cmd->id, result); log_debug(jcon->log, "Success"); - jcon->current = tal_free(cmd); } static void command_fail_v(struct command *cmd, @@ -402,7 +406,6 @@ static void command_fail_v(struct command *cmd, assert(jcon->current == cmd); connection_complete_error(jcon, cmd->id, error, code, data); - jcon->current = tal_free(cmd); } void command_fail(struct command *cmd, const char *fmt, ...) { @@ -628,8 +631,7 @@ static struct io_plan *write_json(struct io_conn *conn, return io_close(conn); } - /* Reader can go again now. */ - io_wake(jcon); + /* Wait for more output. */ return io_out_wait(conn, jcon, write_json, jcon); } diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index 05668b244..aca32e669 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -10,6 +10,7 @@ import queue import os import random import re +import socket import sqlite3 import string import subprocess @@ -3407,6 +3408,24 @@ class LightningDTests(BaseLightningDTests): assert channels[i]['state'] == 'ONCHAIND_MUTUAL' assert channels[-1]['state'] == 'CLOSINGD_COMPLETE' + def test_multirpc(self): + """Test that we can do multiple RPC without waiting for response""" + l1,l2 = self.connect() + + sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + sock.connect(l1.rpc.socket_path) + sock.sendall(b'{"id":1,"jsonrpc":"2.0","method":"listpeers","params":[]}\n' + b'{"id":2,"jsonrpc":"2.0","method":"listpeers","params":[]}\n' + b'{"id":3,"jsonrpc":"2.0","method":"listpeers","params":[]}\n' + b'{"id":4,"jsonrpc":"2.0","method":"listpeers","params":[]}\n' + b'{"id":5,"jsonrpc":"2.0","method":"listpeers","params":[]}\n' + b'{"id":6,"jsonrpc":"2.0","method":"listpeers","params":[]}\n') + + # This isn't quite right, as it will read the first object and stop. + # But enough to trigger the crash! + l1.rpc._readobj(sock) + sock.close() + def test_cli(self): l1 = self.node_factory.get_node()