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 <rusty@rustcorp.com.au>

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);
 	}
 

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()