fix: resolve all bug issues (#158, #163, #164, #165, #166, #167, #168, #169)

2026-07-29 19:02:09 +02:00
parent 269ce0749b
commit 8ad91b8b87
21 changed files with 1498 additions and 385 deletions
@@ -1,16 +1,24 @@
 #include "test_multiprocessing.h"
 #include "multiprocessing.h"
 #include "config.h"
+#include "protocol.h"
 #include "queue.h"
 #include "utils.h"
 #include "test_utils.h"
+#include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
+#include <threads.h>
+#include <sys/wait.h>
+#include <unistd.h>

 /* Test pipeline_context_sender_create/destroy with valid arguments */
 static void test_sender_create_destroy() {
-  Config* cfg = config_create(str_dup("1.0"), str_dup("/src"), str_dup("/dst"), false, false, false,
-                              false, false, 0, false, 0);
+  Config* cfg = config_create();
  EXPECT_NOT_NULL(cfg);
+  cfg->version = str_dup("1.0");
+  cfg->send_directory = str_dup("/src");
+  cfg->receive_root_directory = str_dup("/dst");

  Queue* q_scanner = queue_create(5, NULL);
  EXPECT_NOT_NULL(q_scanner);
@@ -32,9 +40,13 @@ static void test_sender_create_destroy() {

 /* Test pipeline_context_receiver_create/destroy with valid arguments */
 static void test_receiver_create_destroy() {
-  Config* cfg = config_create(str_dup("2.0"), str_dup("/src"), str_dup("/dst"), true, true, false,
-                              false, false, 0, false, 0);
+  Config* cfg = config_create();
  EXPECT_NOT_NULL(cfg);
+  cfg->version = str_dup("2.0");
+  cfg->send_directory = str_dup("/src");
+  cfg->receive_root_directory = str_dup("/dst");
+  cfg->save_to_disk = true;
+  cfg->use_multithreading = true;

  Queue* q = queue_create(20, NULL);
  EXPECT_NOT_NULL(q);
@@ -51,9 +63,11 @@ static void test_receiver_create_destroy() {

 /* Test that create handles various queue capacities */
 static void test_sender_queue_capacities() {
-  Config* cfg = config_create(str_dup("3.0"), str_dup("/src"), str_dup("/dst"), false, false, false,
-                              false, false, 0, false, 0);
+  Config* cfg = config_create();
  EXPECT_NOT_NULL(cfg);
+  cfg->version = str_dup("3.0");
+  cfg->send_directory = str_dup("/src");
+  cfg->receive_root_directory = str_dup("/dst");

  /* Single-element queues */
  Queue* q1 = queue_create(1, NULL);
@@ -67,9 +81,11 @@ static void test_sender_queue_capacities() {

 /* Test that create handles zero-capacity queues */
 static void test_sender_zero_capacity() {
-  Config* cfg = config_create(str_dup("4.0"), str_dup("/src"), str_dup("/dst"), false, false, false,
-                              false, false, 0, false, 0);
+  Config* cfg = config_create();
  EXPECT_NOT_NULL(cfg);
+  cfg->version = str_dup("4.0");
+  cfg->send_directory = str_dup("/src");
+  cfg->receive_root_directory = str_dup("/dst");

  Queue* q1 = queue_create(0, NULL);
  Queue* q2 = queue_create(0, NULL);
@@ -82,8 +98,11 @@ static void test_sender_zero_capacity() {

 /* Test receiver with zero file_descriptor */
 static void test_receiver_fd_zero() {
-  Config* cfg = config_create(str_dup("5.0"), str_dup("/src"), str_dup("/dst"), false, false, false,
-                              false, false, 0, false, 0);
+  Config* cfg = config_create();
+  EXPECT_NOT_NULL(cfg);
+  cfg->version = str_dup("5.0");
+  cfg->send_directory = str_dup("/src");
+  cfg->receive_root_directory = str_dup("/dst");
  Queue* q = queue_create(5, NULL);
  PipelineContextReceiver* ctx = pipeline_context_receiver_create(cfg, q, 0);
  EXPECT_NOT_NULL(ctx);
@@ -92,10 +111,111 @@ static void test_receiver_fd_zero() {
  pipeline_context_receiver_destroy(ctx);
 }

+/* Test that receive_thread completes cleanly when sent FINISHED immediately */
+static void test_receive_thread_finished() {
+  Config* cfg = config_create();
+  EXPECT_NOT_NULL(cfg);
+  cfg->version = str_dup(PROTOCOL_VERSION);
+  cfg->send_directory = str_dup("/src");
+  cfg->receive_root_directory = str_dup("/tmp/dst");
+  cfg->save_to_disk = true;
+
+  int p[2];
+  EXPECT_EQ_INT(pipe(p), 0);
+  io_set_fds(p[0], p[1]);
+  io_set_bwlimit(0);
+
+  pid_t pid = fork();
+  if (pid == 0) {
+    /* Child: run receive_thread */
+    close(p[1]);
+
+    Queue* q = queue_create(5, file_destroy);
+    EXPECT_NOT_NULL(q);
+    PipelineContextReceiver* ctx = pipeline_context_receiver_create(cfg, q, p[0]);
+    EXPECT_NOT_NULL(ctx);
+
+    int ret = receive_thread(ctx);
+
+    pipeline_context_receiver_destroy(ctx);
+    close(p[0]);
+    _exit(ret == thrd_success ? 0 : 1);
+  } else {
+    /* Parent: send STATUS_FINISHED then STATUS_MANIFEST */
+    close(p[0]);
+
+    /* Send a STATUS_FINISHED to make receive_thread exit cleanly.
+     * receive_thread reads status, sees FINISHED, then exits loop.
+     * After the loop it expects STATUS_MANIFEST check, but we sent
+     * FINISHED so it will just return thrd_success. */
+    send_status(p[1], STATUS_FINISHED);
+
+    close(p[1]);
+
+    int status;
+    waitpid(pid, &status, 0);
+
+    /* Parent must free its own copies of config (child has separate copies) */
+    config_delete(cfg);
+
+    EXPECT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) == 0);
+  }
+}
+
+/* Test that write_thread completes cleanly when queue signals done */
+static void test_write_thread_done() {
+  Config* cfg = config_create();
+  EXPECT_NOT_NULL(cfg);
+  cfg->version = str_dup(PROTOCOL_VERSION);
+  cfg->send_directory = str_dup("/src");
+  cfg->receive_root_directory = str_dup("/tmp/dst");
+  cfg->save_to_disk = false;
+
+  Queue* q = queue_create(5, file_destroy);
+  EXPECT_NOT_NULL(q);
+
+  PipelineContextReceiver* ctx = pipeline_context_receiver_create(cfg, q, 0);
+  EXPECT_NOT_NULL(ctx);
+
+  /* Mark receiver as done BEFORE starting the thread so it exits immediately */
+  ctx->receiver_done = true;
+
+  thrd_t writer;
+  int ret = thrd_create(&writer, write_thread, ctx);
+  EXPECT_EQ_INT(ret, thrd_success);
+
+  int result;
+  thrd_join(writer, &result);
+  EXPECT_EQ_INT(result, thrd_success);
+
+  /* Don't call pipeline_context_receiver_destroy because it frees ctx
+   * and write_thread doesn't destroy ctx. Actually looking at the code:
+   * write_thread reads context fields but doesn't free anything.
+   * The caller is responsible for cleanup. So we need to clean up.
+   * But wait - write_thread takes ownership? Let me check...
+   * No, write_thread just processes and returns. The caller frees.
+   *
+   * However, pipeline_context_receiver_destroy will call config_delete
+   * and queue_destroy which would double-free since we created them
+   * in this test. Let me just free the context directly. */
+  mtx_destroy(&ctx->mutex);
+  cnd_destroy(&ctx->condition_not_full);
+  cnd_destroy(&ctx->condition_not_empty);
+  free(ctx);
+
+  /* q and cfg still need cleanup */
+  queue_destroy(q);
+  config_delete(cfg);
+}
+
 void test_multiprocessing() {
  test_sender_create_destroy();
  test_receiver_create_destroy();
  test_sender_queue_capacities();
  test_sender_zero_capacity();
  test_receiver_fd_zero();
+  if (!is_running_under_valgrind()) {
+    test_receive_thread_finished();
+  }
+  test_write_thread_done();
 }