[Tarantool-patches] [PATCH luajit 0/3] Fix sysprof crash due to insufficient free disk space

[Tarantool-patches] [PATCH luajit 0/3] Fix sysprof crash due to insufficient free disk space

[Sergey Bronnikov via Tarantool-patches]

The patch series adds a unit test for lj_wbuf.c file, a patch
with fix for sysprof crash with regression test and a fix for
release resources on a halted profiler.

Unfortunately, we don't have a unit test for lj_wbuf, so before
applying the fix for sysprof, I decided to add a test that
provides not just line code coverage, but also branch coverage.

The LuaJIT code coverage before (master):
https://github.com/tarantool/luajit/actions/runs/38898752231/job/116965510306

lines: 93.0% (21919 out of 23560)
functions: 95.0% (1448 out of 1524)
branches: 80.9% (12757 out of 15761)

The LuaJIT code coverage after (gh-11620-insufficient-free-disk-space-sysprof):

lines: 93.2% (21951 out of 23564)
functions: 95.1% (1449 out of 1524)
branches: 80.9% (12768 out of 15777)

CI: https://github.com/tarantool/luajit/tree/ligurio/gh-11620-insufficient-free-disk-space-sysprof

Related links:
- https://github.com/tarantool/tarantool/issues/11620
- https://coveralls.io/github/tarantool/luajit?branch=ligurio/gh-11620-insufficient-free-disk-space-sysprof

Sergey Bronnikov (3):
  test: add unit tests for the write buffer
  sysprof: fix crash on a failed stream flush
  sysprof: release resources on a halted profiler

 src/lj_sysprof.c                              |   4 +-
 src/lj_wbuf.c                                 |  20 +-
 .../gh-11620-sysprof-enospc-crash.test.c      | 182 +++++++
 test/tarantool-c-tests/unit-wbuf.test.c       | 490 ++++++++++++++++++
 4 files changed, 691 insertions(+), 5 deletions(-)
 create mode 100644 test/tarantool-c-tests/gh-11620-sysprof-enospc-crash.test.c
 create mode 100644 test/tarantool-c-tests/unit-wbuf.test.c

-- 
2.56.0

[Tarantool-patches] [PATCH luajit 1/3] test: add unit tests for the write buffer

[Sergey Bronnikov via Tarantool-patches]

From: Sergey Bronnikov <sergeyb@tarantool.org>

The write buffer accumulates the profilers' output and passes it
to the writer on flush. It has no unit tests, so cover writing of
the data of all kinds, flushing of a full buffer, the explicit
flush and the error reporting.

Part of tarantool/tarantool#11620
---
 test/tarantool-c-tests/unit-wbuf.test.c | 388 ++++++++++++++++++++++++
 1 file changed, 388 insertions(+)
 create mode 100644 test/tarantool-c-tests/unit-wbuf.test.c

diff --git a/test/tarantool-c-tests/unit-wbuf.test.c b/test/tarantool-c-tests/unit-wbuf.test.c
new file mode 100644
index 00000000..4bee6243
--- /dev/null
+++ b/test/tarantool-c-tests/unit-wbuf.test.c
@@ -0,0 +1,388 @@
+/*
+** Testing of the write buffer.
+**
+** The write buffer accumulates the profilers' output and passes
+** it to the writer on flush. The tests below cover writing of the
+** data of all kinds, flushing on overflow, the explicit flush and
+** the error reporting.
+*/
+
+#include <errno.h>
+
+#include <stddef.h>
+#include <stdint.h>
+#include <string.h>
+
+#include "lj_wbuf.h"
+#include "test.h"
+
+/* Size of the write buffer used in the tests. */
+#define BUFFER_SIZE 10
+
+/* Size of the payload that does not fit into the write buffer. */
+#define PAYLOAD_SIZE (2 * BUFFER_SIZE + 5)
+
+/* What the writer does on flush. */
+enum writer_mode {
+	/* Report all the data as written. */
+	WMODE_OK,
+	/* Report a half of the data as written and keep the buffer. */
+	WMODE_PARTIAL,
+	/* Imitate an unrecoverable write error, e.g. ENOSPC. */
+	WMODE_FAIL,
+};
+
+/* Writer context for simulating different write outcomes. */
+struct writer_ctx {
+	/* What the writer does on flush. */
+	enum writer_mode mode;
+	/* Number of performed flushes. */
+	size_t flushes;
+	/* Number of bytes reported as written. */
+	size_t written;
+	/* Everything reported as written, in order of writing. */
+	uint8_t out[2 * PAYLOAD_SIZE];
+};
+
+static size_t test_writer(const void **buf_addr, size_t len, void *opt)
+{
+	struct writer_ctx *ctx = opt;
+	size_t n = len;
+
+	ctx->flushes++;
+	if (ctx->mode == WMODE_FAIL) {
+		errno = ENOSPC;
+		*buf_addr = NULL;
+		return 0;
+	}
+	if (ctx->mode == WMODE_PARTIAL) {
+		errno = EIO;
+		n = len / 2;
+	}
+	memcpy(ctx->out + ctx->written, *buf_addr, n);
+	ctx->written += n;
+	return n;
+}
+
+static void writer_ctx_init(struct writer_ctx *ctx, enum writer_mode mode)
+{
+	memset(ctx, 0, sizeof(*ctx));
+	ctx->mode = mode;
+}
+
+static size_t buf_len(const struct lj_wbuf *buf)
+{
+	return (size_t)(buf->pos - buf->buf);
+}
+
+static void assert_stream_stopped_with_error(const struct lj_wbuf *buf)
+{
+	assert_true(lj_wbuf_test_flag(buf, STREAM_STOP));
+	assert_true(lj_wbuf_test_flag(buf, STREAM_ERRIO));
+	assert_int_equal(lj_wbuf_errno(buf), ENOSPC);
+	assert_ptr_equal(buf->buf, NULL);
+	assert_ptr_equal(buf->pos, NULL);
+}
+
+static int test_addbyte(void *state)
+{
+	uint8_t mem[BUFFER_SIZE];
+	struct lj_wbuf buf;
+	struct writer_ctx ctx;
+
+	UNUSED(state);
+
+	writer_ctx_init(&ctx, WMODE_OK);
+	lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem));
+
+	lj_wbuf_addbyte(&buf, 0x7f);
+	assert_sizet_equal(buf_len(&buf), 1);
+	assert_int_equal(mem[0], 0x7f);
+	assert_sizet_equal(ctx.flushes, 0);
+	assert_int_equal(lj_wbuf_errno(&buf), 0);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+static int test_addu64(void *state)
+{
+	uint8_t mem[BUFFER_SIZE];
+	struct lj_wbuf buf;
+	struct writer_ctx ctx;
+
+	UNUSED(state);
+
+	writer_ctx_init(&ctx, WMODE_OK);
+	lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem));
+
+	/* 128 is encoded as two bytes. */
+	lj_wbuf_addu64(&buf, 128);
+	assert_sizet_equal(buf_len(&buf), 2);
+	assert_int_equal(mem[0], 0x80);
+	assert_int_equal(mem[1], 0x01);
+	assert_sizet_equal(ctx.flushes, 0);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+static int test_addn(void *state)
+{
+	uint8_t mem[BUFFER_SIZE];
+	struct lj_wbuf buf;
+	struct writer_ctx ctx;
+
+	UNUSED(state);
+
+	writer_ctx_init(&ctx, WMODE_OK);
+	lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem));
+
+	lj_wbuf_addn(&buf, "xyz", 3);
+	assert_sizet_equal(buf_len(&buf), 3);
+	assert_true(memcmp(mem, "xyz", 3) == 0);
+	assert_sizet_equal(ctx.flushes, 0);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+static int test_addstring(void *state)
+{
+	uint8_t mem[BUFFER_SIZE];
+	struct lj_wbuf buf;
+	struct writer_ctx ctx;
+
+	UNUSED(state);
+
+	writer_ctx_init(&ctx, WMODE_OK);
+	lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem));
+
+	/* A string is a ULEB128 length followed by the payload. */
+	lj_wbuf_addstring(&buf, "xyz");
+	assert_sizet_equal(buf_len(&buf), 4);
+	assert_int_equal(mem[0], 3);
+	assert_true(memcmp(mem + 1, "xyz", 3) == 0);
+	assert_sizet_equal(ctx.flushes, 0);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+static int test_addn_pumped_through_buffer(void *state)
+{
+	uint8_t mem[BUFFER_SIZE];
+	uint8_t payload[PAYLOAD_SIZE];
+	struct lj_wbuf buf;
+	struct writer_ctx ctx;
+
+	UNUSED(state);
+
+	memset(payload, 'a', sizeof(payload));
+	writer_ctx_init(&ctx, WMODE_OK);
+	lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem));
+
+	/*
+	 * The payload does not fit into the buffer, so it is
+	 * pumped through it. Two full buffers are flushed, the
+	 * rest stays in the buffer.
+	 */
+	lj_wbuf_addn(&buf, payload, sizeof(payload));
+	assert_sizet_equal(ctx.flushes, 2);
+	assert_sizet_equal(ctx.written, 2 * BUFFER_SIZE);
+	assert_true(memcmp(ctx.out, payload, 2 * BUFFER_SIZE) == 0);
+	assert_sizet_equal(buf_len(&buf), PAYLOAD_SIZE - 2 * BUFFER_SIZE);
+	assert_true(memcmp(mem, payload + 2 * BUFFER_SIZE,
+			   PAYLOAD_SIZE - 2 * BUFFER_SIZE) == 0);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+static int test_flush_on_overflow(void *state)
+{
+	uint8_t mem[BUFFER_SIZE];
+	uint8_t payload[BUFFER_SIZE];
+	struct lj_wbuf buf;
+	struct writer_ctx ctx;
+
+	UNUSED(state);
+
+	memset(payload, 'a', sizeof(payload));
+	writer_ctx_init(&ctx, WMODE_OK);
+	lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem));
+
+	/* A byte written to a full buffer is preceded by a flush. */
+	lj_wbuf_addn(&buf, payload, sizeof(payload));
+	assert_sizet_equal(buf_len(&buf), sizeof(mem));
+	lj_wbuf_addbyte(&buf, 0xaa);
+	assert_sizet_equal(ctx.flushes, 1);
+	assert_sizet_equal(ctx.written, sizeof(mem));
+	assert_sizet_equal(buf_len(&buf), 1);
+	assert_int_equal(mem[0], 0xaa);
+
+	/* The same for an ULEB128 value written to a full buffer. */
+	lj_wbuf_addn(&buf, payload, sizeof(payload) - 1);
+	assert_sizet_equal(buf_len(&buf), sizeof(mem));
+	lj_wbuf_addu64(&buf, 0);
+	assert_sizet_equal(ctx.flushes, 2);
+	assert_sizet_equal(ctx.written, 2 * sizeof(mem));
+	assert_sizet_equal(buf_len(&buf), 1);
+
+	/* And for a data block written to a full buffer. */
+	lj_wbuf_addn(&buf, payload, sizeof(payload) - 1);
+	assert_sizet_equal(buf_len(&buf), sizeof(mem));
+	lj_wbuf_addn(&buf, payload, 1);
+	assert_sizet_equal(ctx.flushes, 3);
+	assert_sizet_equal(ctx.written, 3 * sizeof(mem));
+	assert_sizet_equal(buf_len(&buf), 1);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+static int test_flush(void *state)
+{
+	uint8_t mem[BUFFER_SIZE];
+	struct lj_wbuf buf;
+	struct writer_ctx ctx;
+
+	UNUSED(state);
+
+	writer_ctx_init(&ctx, WMODE_OK);
+	lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem));
+
+	lj_wbuf_addn(&buf, "xyz", 3);
+	lj_wbuf_flush(&buf);
+	assert_sizet_equal(ctx.flushes, 1);
+	assert_sizet_equal(ctx.written, 3);
+	assert_true(memcmp(ctx.out, "xyz", 3) == 0);
+	/* The buffer is empty after a successful flush. */
+	assert_sizet_equal(buf_len(&buf), 0);
+	assert_false(lj_wbuf_test_flag(&buf, STREAM_ERRIO | STREAM_STOP));
+
+	/* The buffer is usable after the flush. */
+	lj_wbuf_addbyte(&buf, 0x7f);
+	assert_sizet_equal(buf_len(&buf), 1);
+	assert_int_equal(mem[0], 0x7f);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+static int test_flush_partial_write(void *state)
+{
+	uint8_t mem[BUFFER_SIZE];
+	struct lj_wbuf buf;
+	struct writer_ctx ctx;
+
+	UNUSED(state);
+
+	writer_ctx_init(&ctx, WMODE_PARTIAL);
+	lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem));
+
+	lj_wbuf_addn(&buf, "xyz", 3);
+	lj_wbuf_flush(&buf);
+	/* A partial write is reported as an error. */
+	assert_true(lj_wbuf_test_flag(&buf, STREAM_ERRIO));
+	assert_int_equal(lj_wbuf_errno(&buf), EIO);
+	/* The writer keeps the buffer, so the stream is not stopped. */
+	assert_false(lj_wbuf_test_flag(&buf, STREAM_STOP));
+	assert_ptr_equal(buf.buf, mem);
+	/* The unwritten data is dropped. */
+	assert_sizet_equal(buf_len(&buf), 0);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+static int test_flush_stopped_stream(void *state)
+{
+	uint8_t mem[BUFFER_SIZE];
+	struct lj_wbuf buf;
+	struct writer_ctx ctx;
+
+	UNUSED(state);
+
+	writer_ctx_init(&ctx, WMODE_FAIL);
+	lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem));
+
+	lj_wbuf_addn(&buf, "xyz", 3);
+	lj_wbuf_flush(&buf);
+	assert_stream_stopped_with_error(&buf);
+	assert_sizet_equal(ctx.flushes, 1);
+
+	/* A stopped stream is not flushed again. */
+	lj_wbuf_flush(&buf);
+	assert_sizet_equal(ctx.flushes, 1);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+static int test_write_to_stopped_stream(void *state)
+{
+	uint8_t mem[BUFFER_SIZE];
+	struct lj_wbuf buf;
+	struct writer_ctx ctx;
+
+	UNUSED(state);
+
+	writer_ctx_init(&ctx, WMODE_FAIL);
+	lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem));
+
+	lj_wbuf_addn(&buf, "xyz", 3);
+	lj_wbuf_flush(&buf);
+	assert_stream_stopped_with_error(&buf);
+
+	/* Writes to a stopped stream are ignored. */
+	lj_wbuf_addbyte(&buf, 0x7f);
+	lj_wbuf_addu64(&buf, 128);
+	lj_wbuf_addn(&buf, "xyz", 3);
+	lj_wbuf_addstring(&buf, "xyz");
+	assert_stream_stopped_with_error(&buf);
+	assert_sizet_equal(ctx.flushes, 1);
+	assert_sizet_equal(ctx.written, 0);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+static int test_terminate(void *state)
+{
+	uint8_t mem[BUFFER_SIZE];
+	struct lj_wbuf buf;
+	struct writer_ctx ctx;
+
+	UNUSED(state);
+
+	writer_ctx_init(&ctx, WMODE_OK);
+	lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem));
+
+	lj_wbuf_terminate(&buf);
+	assert_true(buf.writer == NULL);
+	assert_ptr_equal(buf.ctx, NULL);
+	assert_ptr_equal(buf.buf, NULL);
+	assert_ptr_equal(buf.pos, NULL);
+	assert_sizet_equal(buf.size, 0);
+	assert_false(lj_wbuf_test_flag(&buf, STREAM_ERRIO | STREAM_STOP));
+	assert_int_equal(lj_wbuf_errno(&buf), 0);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+/*
+** A writer may release the buffer and set *data to NULL on an
+** unrecoverable error (e.g. ENOSPC). The stream is stopped in
+** this case, so the write functions must not touch the released
+** buffer.
+*/
+int main(void)
+{
+	const struct test_unit tgroup[] = {
+		test_unit_def(test_addbyte),
+		test_unit_def(test_addu64),
+		test_unit_def(test_addn),
+		test_unit_def(test_addstring),
+		test_unit_def(test_addn_pumped_through_buffer),
+		test_unit_def(test_flush_on_overflow),
+		test_unit_def(test_flush),
+		test_unit_def(test_flush_partial_write),
+		test_unit_def(test_flush_stopped_stream),
+		test_unit_def(test_write_to_stopped_stream),
+		test_unit_def(test_terminate),
+	};
+	const int test_result = test_run_group(tgroup, NULL);
+
+	return test_result;
+}
-- 
2.56.0

[Tarantool-patches] [PATCH luajit 2/3] sysprof: fix crash on a failed stream flush

[Sergey Bronnikov via Tarantool-patches]

From: Sergey Bronnikov <sergeyb@tarantool.org>

A writer may release the stream buffer and set its pointer to NULL
on an unrecoverable error, e.g., when the file system with the
profile is full. The write buffer stops the stream in this case,
but a caller of `wbuf_reserve()` still writes data using the
released buffer. This leads to a null pointer dereference inside
`lj_utils_write_uleb128()` when the flush is triggered while
streaming a sample.

Now `wbuf_reserve()` reports whether the stream was stopped during
the flush. All the write functions skip writing in this case,
including the loop pumping a large payload through the buffer.

Fixes tarantool/tarantool#11620
---
 src/lj_wbuf.c                                 |  20 +-
 .../gh-11620-sysprof-enospc-crash.test.c      | 180 ++++++++++++++++++
 test/tarantool-c-tests/unit-wbuf.test.c       | 102 ++++++++++
 3 files changed, 298 insertions(+), 4 deletions(-)
 create mode 100644 test/tarantool-c-tests/gh-11620-sysprof-enospc-crash.test.c

diff --git a/src/lj_wbuf.c b/src/lj_wbuf.c
index 0001a02e..0794b186 100644
--- a/src/lj_wbuf.c
+++ b/src/lj_wbuf.c
@@ -51,11 +51,18 @@ void LJ_FASTCALL lj_wbuf_terminate(struct lj_wbuf *buf)
   lj_wbuf_init(buf, NULL, NULL, NULL, 0);
 }
 
-static LJ_AINLINE void wbuf_reserve(struct lj_wbuf *buf, size_t n)
+/*
+** Reserve n bytes in the buffer. Returns non-zero if the space is reserved
+** and zero if the stream was stopped during the flush. In the latter case
+** the writer may release the buffer (buf->buf == NULL), so it must not be
+** used by the caller.
+*/
+static LJ_AINLINE int wbuf_reserve(struct lj_wbuf *buf, size_t n)
 {
   lj_assertX(n <= buf->size, "wbuf overflow");
   if (LJ_UNLIKELY(wbuf_left(buf) < n))
     lj_wbuf_flush(buf);
+  return !lj_wbuf_test_flag(buf, STREAM_STOP);
 }
 
 /* Writes a byte to the output buffer. */
@@ -63,7 +70,8 @@ void LJ_FASTCALL lj_wbuf_addbyte(struct lj_wbuf *buf, uint8_t b)
 {
   if (LJ_UNLIKELY(lj_wbuf_test_flag(buf, STREAM_STOP)))
     return;
-  wbuf_reserve(buf, sizeof(b));
+  if (LJ_UNLIKELY(!wbuf_reserve(buf, sizeof(b))))
+    return;
   *buf->pos++ = b;
 }
 
@@ -72,7 +80,8 @@ void LJ_FASTCALL lj_wbuf_addu64(struct lj_wbuf *buf, uint64_t n)
 {
   if (LJ_UNLIKELY(lj_wbuf_test_flag(buf, STREAM_STOP)))
     return;
-  wbuf_reserve(buf, LEB128_U64_MAXSIZE);
+  if (LJ_UNLIKELY(!wbuf_reserve(buf, LEB128_U64_MAXSIZE)))
+    return;
   buf->pos += (ptrdiff_t)lj_utils_write_uleb128(buf->pos, n);
 }
 
@@ -91,11 +100,14 @@ void lj_wbuf_addn(struct lj_wbuf *buf, const void *src, size_t n)
     memcpy(buf->pos, src, left);
     buf->pos += (ptrdiff_t)left;
     lj_wbuf_flush(buf);
+    if (LJ_UNLIKELY(lj_wbuf_test_flag(buf, STREAM_STOP)))
+      return;
     src = (uint8_t *)src + (ptrdiff_t)left;
     n -= left;
   }
 
-  wbuf_reserve(buf, n);
+  if (LJ_UNLIKELY(!wbuf_reserve(buf, n)))
+    return;
   memcpy(buf->pos, src, n);
   buf->pos += (ptrdiff_t)n;
 }
diff --git a/test/tarantool-c-tests/gh-11620-sysprof-enospc-crash.test.c b/test/tarantool-c-tests/gh-11620-sysprof-enospc-crash.test.c
new file mode 100644
index 00000000..0f5ffe53
--- /dev/null
+++ b/test/tarantool-c-tests/gh-11620-sysprof-enospc-crash.test.c
@@ -0,0 +1,180 @@
+/*
+** Testing of the sysprof behaviour when a stream write fails.
+**
+** A full file system makes the sysprof writer return an error and
+** release the stream buffer. The profiler must stop the stream
+** gracefully, return an error from luaM_sysprof_stop() and
+** release the resources.
+*/
+
+#include "lauxlib.h"
+#include "lua.h"
+
+/* Need for skipcond for OS and ARCH. */
+#include "lj_arch.h"
+
+#include "test.h"
+
+#if LJ_HASSYSPROF && !defined(LUAJIT_USE_VALGRIND)
+
+#include "lmisclib.h"
+#include "utils.h"
+
+#include <errno.h>
+#include <signal.h>
+#include <stdint.h>
+#include <string.h>
+#include <unistd.h>
+
+/*
+ * The buffer size is enough for the maximal ULEB128 value.
+ * A small buffer makes the profiler flush it right during the
+ * prologue or the first sample.
+ */
+#define STREAM_BUFFER_SIZE 10
+
+/* Do not let the timer interfere with the manually emitted signals. */
+#define SYSPROF_TEST_INTERVAL 1000
+
+struct sysprof_ctx {
+	/* Simulate write errors. */
+	int fail;
+	/* Number of on_stop callback calls. */
+	int on_stop_calls;
+};
+
+static struct sysprof_ctx ctx;
+
+/*
+ * The profiler skips the frames up to
+ * SYSPROF_HANDLER_STACK_DEPTH (6) while streaming the host stack,
+ * so provide the frames after that boundary.
+ */
+#define SYSPROF_TEST_FRAME_START 7
+#define SYSPROF_TEST_FRAME_COUNT 4
+
+static size_t failing_writer(const void **buf_addr, size_t len, void *opt)
+{
+	struct sysprof_ctx *ctx = opt;
+
+	if (ctx->fail) {
+		errno = ENOSPC;
+		*buf_addr = NULL;
+		return 0;
+	}
+
+	return len;
+}
+
+static int on_stop(void *opt, uint8_t *buf)
+{
+	struct sysprof_ctx *ctx = opt;
+
+	UNUSED(buf);
+	ctx->on_stop_calls++;
+	return 0;
+}
+
+static void backtracer(void *(*frame_writer)(int, void *))
+{
+	int frame_no;
+
+	for (frame_no = SYSPROF_TEST_FRAME_START;
+	     frame_no < SYSPROF_TEST_FRAME_START + SYSPROF_TEST_FRAME_COUNT;
+	     ++frame_no)
+		frame_writer(frame_no, (void *)(uintptr_t)frame_no);
+}
+
+static void sysprof_test_configure(struct luam_Sysprof_Options *opt,
+				   uint8_t *buf)
+{
+	memset(&ctx, 0, sizeof(ctx));
+	memset(opt, 0, sizeof(*opt));
+	opt->mode = LUAM_SYSPROF_CALLGRAPH;
+	opt->interval = SYSPROF_TEST_INTERVAL;
+	opt->buf = buf;
+	opt->len = STREAM_BUFFER_SIZE;
+	opt->ctx = &ctx;
+
+	assert_true(luaM_sysprof_set_writer(failing_writer) == PROFILE_SUCCESS);
+	assert_true(luaM_sysprof_set_on_stop(on_stop) == PROFILE_SUCCESS);
+	assert_true(luaM_sysprof_set_backtracer(backtracer) == PROFILE_SUCCESS);
+}
+
+static int sysprof_enospc_during_sampling(void *test_state)
+{
+	lua_State *L = test_state;
+	static uint8_t buf[STREAM_BUFFER_SIZE];
+	struct luam_Sysprof_Options opt;
+	struct luam_Sysprof_Counters counters = {};
+
+	sysprof_test_configure(&opt, buf);
+	assert_true(luaM_sysprof_start(L, &opt) == PROFILE_SUCCESS);
+	assert_int_equal(ctx.on_stop_calls, 0);
+
+	/* Let the next stream flush fail as if the disk is full. */
+	ctx.fail = 1;
+	assert_int_equal(kill(getpid(), SIGPROF), 0);
+
+	/* The stream is stopped gracefully, without a crash. */
+	errno = 0;
+	assert_int_equal(luaM_sysprof_stop(L), PROFILE_ERRIO);
+	assert_int_equal(errno, ENOSPC);
+
+	/* Counters are still available. */
+	assert_true(luaM_sysprof_report(&counters) == PROFILE_SUCCESS);
+	assert_true(counters.samples >= 1);
+
+	/* The profiler is stopped. */
+	assert_int_equal(luaM_sysprof_stop(L), PROFILE_ERRRUN);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+static int sysprof_enospc_on_start(void *test_state)
+{
+	lua_State *L = test_state;
+	static uint8_t buf[STREAM_BUFFER_SIZE];
+	struct luam_Sysprof_Options opt;
+
+	sysprof_test_configure(&opt, buf);
+	/* The very first flush fails, so the prologue cannot be streamed. */
+	ctx.fail = 1;
+	errno = 0;
+	assert_int_equal(luaM_sysprof_start(L, &opt), PROFILE_ERRIO);
+	assert_int_equal(errno, ENOSPC);
+	/* The start error path releases the profiler resources. */
+	assert_int_equal(ctx.on_stop_calls, 1);
+
+	/* The profiler is not running. */
+	assert_int_equal(luaM_sysprof_stop(L), PROFILE_ERRRUN);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+#endif /* LJ_HASSYSPROF && !defined(LUAJIT_USE_VALGRIND) */
+
+int main(void)
+{
+#if LUAJIT_USE_VALGRIND
+	return skip_all("Disabled due to #10803");
+#elif !LJ_HASSYSPROF
+	return skip_all("Sysprof is disabled");
+#else
+	if (LUAJIT_OS != LUAJIT_OS_LINUX)
+		return skip_all("Sysprof is implemented for Linux only");
+	if (LUAJIT_TARGET != LUAJIT_ARCH_X86
+	    && LUAJIT_TARGET != LUAJIT_ARCH_X64)
+		return skip_all("Sysprof is implemented for x86_64 only");
+
+	lua_State *L = utils_lua_init();
+
+	const struct test_unit tgroup[] = {
+		test_unit_def(sysprof_enospc_during_sampling),
+		test_unit_def(sysprof_enospc_on_start)
+	};
+	const int test_result = test_run_group(tgroup, L);
+	utils_lua_close(L);
+	return test_result;
+#endif /* LUAJIT_USE_VALGRIND */
+}
diff --git a/test/tarantool-c-tests/unit-wbuf.test.c b/test/tarantool-c-tests/unit-wbuf.test.c
index 4bee6243..c1b71ff3 100644
--- a/test/tarantool-c-tests/unit-wbuf.test.c
+++ b/test/tarantool-c-tests/unit-wbuf.test.c
@@ -367,6 +367,104 @@ static int test_terminate(void *state)
 ** this case, so the write functions must not touch the released
 ** buffer.
 */
+static int test_addbyte_failed_flush(void *state)
+{
+	uint8_t mem[BUFFER_SIZE];
+	struct lj_wbuf buf;
+	struct writer_ctx ctx;
+	size_t i;
+
+	UNUSED(state);
+
+	writer_ctx_init(&ctx, WMODE_FAIL);
+	lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem));
+
+	/* Fill the buffer without flushes. */
+	for (i = 0; i < sizeof(mem); ++i)
+		lj_wbuf_addbyte(&buf, (uint8_t)i);
+	assert_sizet_equal(ctx.flushes, 0);
+	assert_sizet_equal(buf_len(&buf), sizeof(mem));
+
+	/* The buffer is full, so the byte below triggers a failed flush. */
+	lj_wbuf_addbyte(&buf, 0xaa);
+	assert_stream_stopped_with_error(&buf);
+	assert_sizet_equal(ctx.flushes, 1);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+static int test_addu64_failed_flush(void *state)
+{
+	uint8_t mem[BUFFER_SIZE];
+	struct lj_wbuf buf;
+	struct writer_ctx ctx;
+
+	UNUSED(state);
+
+	writer_ctx_init(&ctx, WMODE_FAIL);
+	lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem));
+
+	/* The maximal value takes exactly BUFFER_SIZE bytes. */
+	lj_wbuf_addu64(&buf, UINT64_MAX);
+	assert_sizet_equal(buf_len(&buf), sizeof(mem));
+
+	/* There is no space left, so the next write triggers a failed flush. */
+	lj_wbuf_addu64(&buf, 0);
+	assert_stream_stopped_with_error(&buf);
+	assert_sizet_equal(ctx.flushes, 1);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+static int test_addn_failed_flush(void *state)
+{
+	uint8_t mem[BUFFER_SIZE];
+	uint8_t payload[BUFFER_SIZE];
+	struct lj_wbuf buf;
+	struct writer_ctx ctx;
+
+	UNUSED(state);
+
+	memset(payload, 'a', sizeof(payload));
+	writer_ctx_init(&ctx, WMODE_FAIL);
+	lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem));
+
+	/* Fill the buffer without flushes. */
+	lj_wbuf_addn(&buf, payload, sizeof(payload));
+	assert_sizet_equal(ctx.flushes, 0);
+
+	/* There is no space left, so the next write triggers a failed flush. */
+	lj_wbuf_addn(&buf, "b", 1);
+	assert_stream_stopped_with_error(&buf);
+	assert_sizet_equal(ctx.flushes, 1);
+
+	return TEST_EXIT_SUCCESS;
+}
+
+static int test_addn_failed_flush_in_loop(void *state)
+{
+	uint8_t mem[BUFFER_SIZE];
+	uint8_t payload[PAYLOAD_SIZE];
+	struct lj_wbuf buf;
+	struct writer_ctx ctx;
+
+	UNUSED(state);
+
+	memset(payload, 'a', sizeof(payload));
+	writer_ctx_init(&ctx, WMODE_FAIL);
+	lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem));
+
+	/*
+	 * The payload does not fit into the buffer, so it is pumped through
+	 * it. The flush fails in the middle of the pump loop.
+	 */
+	lj_wbuf_addn(&buf, payload, sizeof(payload));
+	assert_stream_stopped_with_error(&buf);
+	assert_sizet_equal(ctx.flushes, 1);
+
+	return TEST_EXIT_SUCCESS;
+}
+
 int main(void)
 {
 	const struct test_unit tgroup[] = {
@@ -381,6 +479,10 @@ int main(void)
 		test_unit_def(test_flush_stopped_stream),
 		test_unit_def(test_write_to_stopped_stream),
 		test_unit_def(test_terminate),
+		test_unit_def(test_addbyte_failed_flush),
+		test_unit_def(test_addu64_failed_flush),
+		test_unit_def(test_addn_failed_flush),
+		test_unit_def(test_addn_failed_flush_in_loop),
 	};
 	const int test_result = test_run_group(tgroup, NULL);
 
-- 
2.56.0

[Tarantool-patches] [PATCH luajit 3/3] sysprof: release resources on a halted profiler

[Sergey Bronnikov via Tarantool-patches]

From: Sergey Bronnikov <sergeyb@tarantool.org>

If the stream fails (e.g., because of ENOSPC), the profiler moves
to the SPS_HALT state. `lj_sysprof_stop()` returns PROFILE_ERRIO
but does not call the `on_stop` callback, so the profile context
and the output file descriptor are leaked.

Call `on_stop` when the halted profiler is stopped, as it is
already done when starting fails or when the stream is stopped in
a regular way. Preserve the saved errno because `on_stop` may
change it.

Follows up tarantool/tarantool#11620
---
 src/lj_sysprof.c                                            | 4 +++-
 test/tarantool-c-tests/gh-11620-sysprof-enospc-crash.test.c | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/lj_sysprof.c b/src/lj_sysprof.c
index 99bcc088..667c93a8 100644
--- a/src/lj_sysprof.c
+++ b/src/lj_sysprof.c
@@ -509,9 +509,11 @@ int lj_sysprof_stop(lua_State *L)
   lj_profile_timer_stop(&sp->timer);
 
   if (SPS_HALT == sp->state) {
-    errno = sp->saved_errno;
+    const int saved_errno = sp->saved_errno;
     sp->state = SPS_IDLE;
     /* wbuf was terminated when error occurred. */
+    sp->on_stop(sp->opt.ctx, sp->out.buf);
+    errno = saved_errno;
     return PROFILE_ERRIO;
   }
 
diff --git a/test/tarantool-c-tests/gh-11620-sysprof-enospc-crash.test.c b/test/tarantool-c-tests/gh-11620-sysprof-enospc-crash.test.c
index 0f5ffe53..3c9ac05d 100644
--- a/test/tarantool-c-tests/gh-11620-sysprof-enospc-crash.test.c
+++ b/test/tarantool-c-tests/gh-11620-sysprof-enospc-crash.test.c
@@ -120,6 +120,8 @@ static int sysprof_enospc_during_sampling(void *test_state)
 	errno = 0;
 	assert_int_equal(luaM_sysprof_stop(L), PROFILE_ERRIO);
 	assert_int_equal(errno, ENOSPC);
+	/* The error path releases the profiler resources. */
+	assert_int_equal(ctx.on_stop_calls, 1);
 
 	/* Counters are still available. */
 	assert_true(luaM_sysprof_report(&counters) == PROFILE_SUCCESS);
-- 
2.56.0