Skip to content

Conversation

@cosmo0920
Copy link
Contributor

@cosmo0920 cosmo0920 commented Nov 6, 2025

Fixes #11104.


Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • Example configuration file for the change
service:
    flush_interval: 1
    daemon: Off
    log_level: debug

pipeline:
    inputs:
        -   name: tcp
            listen: 0.0.0.0
            port: 24224
            format: json
            tag: input.json_tcp
            buffer_size: 128 #kb

    outputs:
        -   name: s3
            match: 'common_output'
            region: eu-west-1
            bucket: TEST
            store_dir_limit_size: 200M
            compression: gzip
            total_file_size: 50M
            s3_key_format: "/%Y/%m/%d/%H_%M_%S_$UUID.json"
  • Debug log output from testing the change
% bin/fluent-bit -c out_s3_segv.yaml --dry-run
Fluent Bit v4.2.0
* Copyright (C) 2015-2025 The Fluent Bit Authors
* Fluent Bit is a CNCF sub-project under the umbrella of Fluentd
* https://fluentbit.io

______ _                  _    ______ _ _             ___   __  
|  ___| |                | |   | ___ (_) |           /   | /  | 
| |_  | |_   _  ___ _ __ | |_  | |_/ /_| |_  __   __/ /| | `| | 
|  _| | | | | |/ _ \ '_ \| __| | ___ \ | __| \ \ / / /_| |  | | 
| |   | | |_| |  __/ | | | |_  | |_/ / | |_   \ V /\___  |__| |_
\_|   |_|\__,_|\___|_| |_|\__| \____/|_|\__|   \_/     |_(_)___/


[2025/11/06 19:44:26.316110000] [ info] Configuration:
[2025/11/06 19:44:26.316116000] [ info]  flush time     | 1.000000 seconds
[2025/11/06 19:44:26.316120000] [ info]  grace          | 5 seconds
[2025/11/06 19:44:26.316121000] [ info]  daemon         | 0
[2025/11/06 19:44:26.316123000] [ info] ___________
[2025/11/06 19:44:26.316124000] [ info]  inputs:
[2025/11/06 19:44:26.316126000] [ info]      tcp
[2025/11/06 19:44:26.316128000] [ info] ___________
[2025/11/06 19:44:26.316129000] [ info]  filters:
[2025/11/06 19:44:26.316131000] [ info] ___________
[2025/11/06 19:44:26.316132000] [ info]  outputs:
[2025/11/06 19:44:26.316134000] [ info]      s3.0
[2025/11/06 19:44:26.316135000] [ info] ___________
[2025/11/06 19:44:26.316137000] [ info]  collectors:
configuration test is successful
  • Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • Run local packaging test showing all targets (including any new ones) build.
  • Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • Documentation required for this feature

Backporting

  • Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced the S3 output plugin's worker termination process with additional defensive checks to prevent crashes and ensure stable plugin shutdown.
    • Improved logging accuracy during worker lifecycle management to clearly distinguish between initialization and termination phases, enabling better operational visibility and troubleshooting.

@coderabbitai
Copy link

coderabbitai bot commented Nov 6, 2025

Walkthrough

A NULL context guard is added to the S3 worker exit handler to prevent an invalid memory access during shutdown. The change intercepts early returns when the context is NULL, resolving a segmentation fault that occurs during --dry-run with S3 output enabled.

Changes

Cohort / File(s) Summary
S3 Worker Exit Safety
plugins/out_s3/s3.c
Added NULL pointer guard in cb_s3_worker_exit to return early if context is NULL; updated log message from "initializing worker" to "terminating worker"; maintains existing cleanup behavior for non-NULL context.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Straightforward NULL check guard addition preventing invalid memory access
  • Single file, localized change with clear intent tied to the shutdown path
  • Log message update is cosmetic and contextually appropriate

Suggested reviewers

  • edsiper
  • koleini
  • fujimotos

Poem

A worker exits, but context falls through,
A NULL check catches what chaos could brew,
The rabbit hops safely, no segfault in sight,
One guard does the trick—dry-run's now alright! 🐰✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR addresses the core requirement from #11104 by adding a NULL guard in cb_s3_worker_exit to prevent invalid memory access, but does not implement the requested test coverage for dry-run scenarios. Add a test case that verifies --dry-run with s3 output enabled does not segfault, as requested in the linked issue's expected behavior section.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a NULL check in the s3 plugin to prevent a segmentation fault during dry-run operations.
Out of Scope Changes check ✅ Passed The changes remain focused on the reported issue: adding NULL check and updating log message in cb_s3_worker_exit as a targeted fix for the segfault.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch cosmo0920-plug-segv-without-null-check

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
plugins/out_s3/s3.c (1)

1115-1119: LGTM! Critical NULL check fixes the dry-run segfault.

The NULL guard directly addresses issue #11104 by preventing the invalid memory access to ctx->ins at line 1119 that caused the segfault during --dry-run. Returning early is appropriate for this cleanup function.

Optional refinement: The TLS cleanup code at lines 1121-1125 doesn't depend on ctx, so theoretically it could still execute even when ctx is NULL:

 static int cb_s3_worker_exit(void *data, struct flb_config *config)
 {
     struct worker_info *info;
     struct flb_s3 *ctx = data;
 
-    if (!ctx) {
-        return 0;
-    }
-
-    flb_plg_info(ctx->ins, "terminating worker");
+    if (ctx) {
+        flb_plg_info(ctx->ins, "terminating worker");
+    }
 
     info = FLB_TLS_GET(s3_worker_info);
     if (info != NULL) {
         flb_free(info);
         FLB_TLS_SET(s3_worker_info, NULL);
     }
 
     return 0;
 }

However, the current implementation is perfectly fine—if ctx is NULL during dry-run, the worker was likely never fully initialized, so skipping TLS cleanup is reasonable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 02c9e9f and 896b8fb.

📒 Files selected for processing (1)
  • plugins/out_s3/s3.c (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)
  • GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
  • GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
  • GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
  • GitHub Check: PR - fuzzing test
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
  • GitHub Check: pr-compile-centos-7
  • GitHub Check: pr-compile-without-cxx (3.31.6)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dry-run segfaults on linux if config has s3 output

3 participants