Abandon a fill whose last flush fails to write

The slot being filled was forgotten before the page's final flush, so a
write that failed there, on a full disk for one, could not abandon the
fill: cgit died into the lock file instead and the visitor got nothing.
The mid-page path already delivered the page from the lock file.

2 files changed, 20 insertions, 2 deletions

diff --git a/source/cache.c b/source/cache.cindex a076823..82b3aad 100644--- a/source/cache.c+++ b/source/cache.c@@ -370,12 +370,14 @@ static int fill_slot(struct cache_slot *slot)  	slot_being_filled = slot; 	slot->fn();-	slot_being_filled = NULL;  	// All of the page has to reach the lock file before it is renamed-	// into place. After an abandoned fill stdout is the client again and+	// into place, and a write that fails on this last flush still finds+	// the slot to abandon, so the page goes to the visitor rather than+	// nowhere. After an abandoned fill stdout is the client again and 	// this same flush delivers the tail of the error page instead. 	html_flush();+	slot_being_filled = NULL; 	if (fflush(stdout)) 		return errno; 
diff --git a/source/cache.c b/source/cache.c
index a076823..82b3aad 100644
--- a/source/cache.c
+++ b/source/cache.c
@@ -370,12 +370,14 @@ static int fill_slot(struct cache_slot *slot)
 
 	slot_being_filled = slot;
 	slot->fn();
-	slot_being_filled = NULL;
 
 	// All of the page has to reach the lock file before it is renamed
-	// into place. After an abandoned fill stdout is the client again and
+	// into place, and a write that fails on this last flush still finds
+	// the slot to abandon, so the page goes to the visitor rather than
+	// nowhere. After an abandoned fill stdout is the client again and
 	// this same flush delivers the tail of the error page instead.
 	html_flush();
+	slot_being_filled = NULL;
 	if (fflush(stdout))
 		return errno;
 
diff --git a/tests/t0003-cache.sh b/tests/t0003-cache.shindex 4b6a43c..bd186e7 100755--- a/tests/t0003-cache.sh+++ b/tests/t0003-cache.sh@@ -227,4 +227,20 @@ test_expect_success 'pages following a branch take the dynamic ttl' ' 	test_line_count = 2 summary.slots ' +# A write into the lock file that fails on the page's last flush has to end+# as one that fails mid page does, with the fill abandoned and the page+# delivered from the lock file, rather than in a blank response. The limit+# is on files alone, so the page goes to the visitor through a pipe.+test_expect_success 'a failed final write still delivers the page' '+	rm -rf cache3 && mkdir cache3 &&+	(+		trap "" XFSZ &&+		ulimit -f 2 &&+		CGIT_CONFIG="$PWD/bigkeyrc" QUERY_STRING="url=bigpage/" cgit 2>flush.err+	) | cat >flush.out &&+	grep "^Status: 200" flush.out &&+	grep "</html>" flush.out &&+	! grep "called many times" flush.err+'+ test_done
diff --git a/tests/t0003-cache.sh b/tests/t0003-cache.sh
index 4b6a43c..bd186e7 100755
--- a/tests/t0003-cache.sh
+++ b/tests/t0003-cache.sh
@@ -227,4 +227,20 @@ test_expect_success 'pages following a branch take the dynamic ttl' '
 	test_line_count = 2 summary.slots
 '
 
+# A write into the lock file that fails on the page's last flush has to end
+# as one that fails mid page does, with the fill abandoned and the page
+# delivered from the lock file, rather than in a blank response. The limit
+# is on files alone, so the page goes to the visitor through a pipe.
+test_expect_success 'a failed final write still delivers the page' '
+	rm -rf cache3 && mkdir cache3 &&
+	(
+		trap "" XFSZ &&
+		ulimit -f 2 &&
+		CGIT_CONFIG="$PWD/bigkeyrc" QUERY_STRING="url=bigpage/" cgit 2>flush.err
+	) | cat >flush.out &&
+	grep "^Status: 200" flush.out &&
+	grep "</html>" flush.out &&
+	! grep "called many times" flush.err
+'
+
 test_done