logging: always dump a crash log, but make files per-pid.

Someone had a 21GB crash.log, which doesn't help anyone!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd/log.c

@@ -548,9 +548,6 @@ static void log_dump_to_file(int fd, const struct log_book *lr)
 	struct log_data data;
 	time_t start;
 
-	write_all(fd, "Start of new crash log\n",
-		  strlen("Start of new crash log\n"));
-
 	i = list_top(&lr->log, const struct log_entry, list);
 	if (!i) {
 		write_all(fd, "0 bytes:\n\n", strlen("0 bytes:\n\n"));
@@ -571,28 +568,27 @@ static void log_dump_to_file(int fd, const struct log_book *lr)
 /* FIXME: Dump peer logs! */
 void log_backtrace_exit(void)
 {
+	int fd;
+	char logfile[sizeof("/tmp/lightning-crash.log.%u") + STR_MAX_CHARS(int)];
+
 	if (!crashlog)
 		return;
 
-	/* If we're not already pointing at a log file, make one */
-	if (crashlog->lr->print == log_to_stdout) {
-		const char *logfile = NULL;
-		int fd;
-
-		/* We expect to be in config dir. */
-		logfile = "crash.log";
-		fd = open(logfile, O_WRONLY|O_CREAT|O_APPEND, 0600);
-		if (fd < 0) {
-			logfile = "/tmp/lightning-crash.log";
-			fd = open(logfile, O_WRONLY|O_CREAT, 0600);
-		}
+	/* We expect to be in config dir. */
+	snprintf(logfile, sizeof(logfile), "crash.log.%u", getpid());
 
-		/* Dump entire log. */
-		if (fd >= 0) {
-			log_dump_to_file(fd, crashlog->lr);
-			close(fd);
-			fprintf(stderr, "Log dumped in %s\n", logfile);
-		}
+	fd = open(logfile, O_WRONLY|O_CREAT|O_TRUNC, 0600);
+	if (fd < 0) {
+		snprintf(logfile, sizeof(logfile),
+			 "/tmp/lightning-crash.log.%u", getpid());
+		fd = open(logfile, O_WRONLY|O_CREAT|O_TRUNC, 0600);
+	}
+
+	/* Dump entire log. */
+	if (fd >= 0) {
+		log_dump_to_file(fd, crashlog->lr);
+		close(fd);
+		fprintf(stderr, "Log dumped in %s\n", logfile);
 	}
 }
 

tests/test_misc.py

@@ -2,7 +2,7 @@ from decimal import Decimal
 from fixtures import *  # noqa: F401,F403
 from flaky import flaky
 from lightning import RpcError
-from utils import DEVELOPER, sync_blockheight, only_one, wait_for, TailableProc
+from utils import DEVELOPER, VALGRIND, sync_blockheight, only_one, wait_for, TailableProc
 from ephemeral_port_reserve import reserve
 
 import json
@@ -868,3 +868,15 @@ def test_logging(node_factory):
 
     assert log1[-1].endswith("Ending log due to SIGHUP\n")
     assert log2[0].endswith("Started log due to SIGHUP\n")
+
+
+@unittest.skipIf(VALGRIND and not DEVELOPER,
+                 "Backtrace upsets valgrind: only suppressed in DEVELOPER mode")
+def test_crashlog(node_factory):
+    l1 = node_factory.get_node(may_fail=True)
+
+    crashpath = os.path.join(l1.daemon.lightning_dir,
+                             'crash.log.{}'.format(l1.daemon.proc.pid))
+    assert not os.path.exists(crashpath)
+    l1.daemon.proc.send_signal(signal.SIGSEGV)
+    wait_for(lambda: os.path.exists(crashpath))