Merged
Conversation
Reviewer's GuideImplements standard Testcontainers labels and session ID support, ensuring all containers and networks created by the library are tagged with a consistent set of metadata, and exposes related constants and helpers via the public API with accompanying tests. Sequence diagram for container creation label injectionsequenceDiagram
actor TestCode
participant DockerClient
participant JsonBuilder
participant SessionIdCache
participant StdCryptoRandom
TestCode->>DockerClient: buildCreateBody(allocator, containerRequest)
DockerClient->>JsonBuilder: init root JSON object
DockerClient->>JsonBuilder: init lbl_obj ObjectMap
DockerClient->>DockerClient: putDefaultLabels(lbl_obj)
activate DockerClient
DockerClient->>SessionIdCache: getSessionId()
alt first_call
SessionIdCache->>StdCryptoRandom: bytes(random_bytes[16])
StdCryptoRandom-->>SessionIdCache: random_bytes
SessionIdCache->>SessionIdCache: set UUID version and variant bits
SessionIdCache->>SessionIdCache: formatUuidV4(session_id_buf, random_bytes)
SessionIdCache->>SessionIdCache: session_id_initialized = true
SessionIdCache-->>DockerClient: session_id_buf
else cached_value
SessionIdCache-->>DockerClient: session_id_buf
end
DockerClient->>JsonBuilder: put(label_base, "true")
DockerClient->>JsonBuilder: put(label_lang, "zig")
DockerClient->>JsonBuilder: put(label_version, tc_version)
DockerClient->>JsonBuilder: put(label_session_id, session_id_buf)
deactivate DockerClient
loop for each user_label in containerRequest.labels
DockerClient->>JsonBuilder: put(user_label.key, user_label.value)
end
DockerClient->>JsonBuilder: root.object.put(Labels, lbl_obj)
DockerClient-->>TestCode: JSON body with merged labels
Sequence diagram for network creation label injectionsequenceDiagram
actor TestCode
participant DockerClient
participant JsonBuilder
participant SessionIdCache
TestCode->>DockerClient: buildNetworkCreateBody(allocator, name, driver, labels)
DockerClient->>JsonBuilder: init root JSON object
DockerClient->>JsonBuilder: put(Driver, driver)
DockerClient->>JsonBuilder: put(CheckDuplicate, true)
DockerClient->>JsonBuilder: init lbl_obj ObjectMap
DockerClient->>DockerClient: putDefaultLabels(lbl_obj)
DockerClient->>SessionIdCache: getSessionId()
SessionIdCache-->>DockerClient: cached session_id_buf
loop for each user_label in labels
DockerClient->>JsonBuilder: put(user_label.key, user_label.value)
end
DockerClient->>JsonBuilder: root.object.put(Labels, lbl_obj)
DockerClient-->>TestCode: JSON body with merged labels
Class diagram for docker_client label and session ID helpersclassDiagram
class docker_client_module {
<<module>>
+const docker_socket : [*]const u8
+const api_version : [*]const u8
+const label_base : [*]const u8
+const label_lang : [*]const u8
+const label_version : [*]const u8
+const label_session_id : [*]const u8
+const tc_version : [*]const u8
-var session_id_buf : [36]u8
-var session_id_initialized : bool
-fn formatUuidV4(buf : *[36]u8, bytes : [16]u8) void
+fn getSessionId() []const u8
-fn putDefaultLabels(obj : *std_json_ObjectMap) !void
+fn buildCreateBody(allocator : std_mem_Allocator, req : *const container_mod_ContainerRequest) ![]u8
+fn buildNetworkCreateBody(allocator : std_mem_Allocator, name : []const u8, driver : []const u8, labels : []const container_mod_KV) ![]u8
}
class root_module {
<<module>>
+const DockerClient : type
+const docker_socket : [*]const u8
+const label_base : [*]const u8
+const label_lang : [*]const u8
+const label_version : [*]const u8
+const label_session_id : [*]const u8
+const tc_version : [*]const u8
+fn getSessionId() []const u8
}
docker_client_module <.. root_module : reexports
class std_json_ObjectMap {
<<struct>>
+fn put(key : []const u8, value : std_json_Value) !void
}
class container_mod_ContainerRequest {
<<struct>>
+image : []const u8
+labels : []const container_mod_KV
}
class container_mod_KV {
<<struct>>
+key : []const u8
+value : []const u8
}
class std_mem_Allocator {
<<struct>>
}
docker_client_module --> std_json_ObjectMap : uses
docker_client_module --> container_mod_ContainerRequest : uses
docker_client_module --> container_mod_KV : uses
docker_client_module --> std_mem_Allocator : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
📝 WalkthroughWalkthroughIntroduced standardized Testcontainers label support for containers and networks. Added public label constants (base, language, version, session ID) and a session ID accessor that generates and caches UUID v4 strings. Updated container/network creation logic to inject default labels and merge with user-supplied labels. Extended public API exports and test coverage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The lazy session ID caching uses module-level mutable state (
session_id_initializedandsession_id_buf) without synchronization, which could lead to data races if the library is used from multiple threads; consider making initialization thread-safe (e.g. viastd.Thread.Mutexorstd.once-style logic). - The hardcoded
tc_version = "0.1.0"risks drifting from the actual library version; it might be more robust to centralize this in a single version definition or derive it from build metadata so updates don’t require touching this constant.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The lazy session ID caching uses module-level mutable state (`session_id_initialized` and `session_id_buf`) without synchronization, which could lead to data races if the library is used from multiple threads; consider making initialization thread-safe (e.g. via `std.Thread.Mutex` or `std.once`-style logic).
- The hardcoded `tc_version = "0.1.0"` risks drifting from the actual library version; it might be more robust to centralize this in a single version definition or derive it from build metadata so updates don’t require touching this constant.
## Individual Comments
### Comment 1
<location path="src/docker_client.zig" line_range="62-71" />
<code_context>
+
+/// Returns the per-process session ID as a UUID v4 string.
+/// The value is generated lazily on first call and cached.
+pub fn getSessionId() []const u8 {
+ if (!session_id_initialized) {
+ var bytes: [16]u8 = undefined;
+ std.crypto.random.bytes(&bytes);
+ // UUID v4: set version nibble to 4, variant bits to 10xx
+ bytes[6] = (bytes[6] & 0x0f) | 0x40;
+ bytes[8] = (bytes[8] & 0x3f) | 0x80;
+ formatUuidV4(&session_id_buf, bytes);
+ session_id_initialized = true;
+ }
+ return &session_id_buf;
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Session ID lazy init is not thread-safe and can race under concurrent access.
`session_id_initialized` and `session_id_buf` are written concurrently with no synchronization, so multiple threads can enter the initialization path and write overlapping random data, producing corrupted UUIDs. Guard this with a one-time init mechanism such as `std.Thread.once`, an atomic boolean, or another primitive that ensures only a single thread performs initialization and others only read the fully initialized buffer.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+62
to
+71
| pub fn getSessionId() []const u8 { | ||
| if (!session_id_initialized) { | ||
| var bytes: [16]u8 = undefined; | ||
| std.crypto.random.bytes(&bytes); | ||
| // UUID v4: set version nibble to 4, variant bits to 10xx | ||
| bytes[6] = (bytes[6] & 0x0f) | 0x40; | ||
| bytes[8] = (bytes[8] & 0x3f) | 0x80; | ||
| formatUuidV4(&session_id_buf, bytes); | ||
| session_id_initialized = true; | ||
| } |
There was a problem hiding this comment.
issue (bug_risk): Session ID lazy init is not thread-safe and can race under concurrent access.
session_id_initialized and session_id_buf are written concurrently with no synchronization, so multiple threads can enter the initialization path and write overlapping random data, producing corrupted UUIDs. Guard this with a one-time init mechanism such as std.Thread.once, an atomic boolean, or another primitive that ensures only a single thread performs initialization and others only read the fully initialized buffer.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/docker_client.zig`:
- Around line 57-73: The lazy initialization of session_id_buf in getSessionId
is not thread-safe; wrap the initialization with a synchronization primitive so
only one thread generates and writes the UUID and others wait/see the
initialized value. Replace the plain bool session_id_initialized and unprotected
write to session_id_buf with either a mutex (std.Thread.Mutex) that locks around
the generation and call to formatUuidV4 inside getSessionId, or use an atomic
flag (std.atomic.AtomicBool or `@atomicCompareExchange` on session_id_initialized)
to perform a compare-and-swap "once" and ensure writers coordinate before
writing session_id_buf; update callers to return the buffer after the protected
initialization and ensure session_id_buf remains a stable global.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f6cdbbd0-d273-4e84-bca8-ff93560ddb15
📒 Files selected for processing (3)
src/docker_client.zigsrc/integration_test.zigsrc/root.zig
Comment on lines
+57
to
+73
| var session_id_buf: [36]u8 = undefined; | ||
| var session_id_initialized: bool = false; | ||
|
|
||
| /// Returns the per-process session ID as a UUID v4 string. | ||
| /// The value is generated lazily on first call and cached. | ||
| pub fn getSessionId() []const u8 { | ||
| if (!session_id_initialized) { | ||
| var bytes: [16]u8 = undefined; | ||
| std.crypto.random.bytes(&bytes); | ||
| // UUID v4: set version nibble to 4, variant bits to 10xx | ||
| bytes[6] = (bytes[6] & 0x0f) | 0x40; | ||
| bytes[8] = (bytes[8] & 0x3f) | 0x80; | ||
| formatUuidV4(&session_id_buf, bytes); | ||
| session_id_initialized = true; | ||
| } | ||
| return &session_id_buf; | ||
| } |
Contributor
There was a problem hiding this comment.
Race condition in lazy session ID initialization.
session_id_initialized and session_id_buf are global mutable state accessed without synchronization. If multiple threads call getSessionId() concurrently (e.g., in a multi-threaded application creating containers in parallel), the check-then-set pattern can result in:
- Multiple threads generating different UUIDs
- Torn writes to
session_id_buf - Non-deterministic session IDs across containers
Consider using std.Thread.Mutex or @atomicStore/@atomicLoad for thread-safe initialization.
🔒 Proposed fix using atomic once-initialization pattern
-var session_id_buf: [36]u8 = undefined;
-var session_id_initialized: bool = false;
+var session_id_buf: [36]u8 = undefined;
+var session_id_state: std.atomic.Value(u8) = std.atomic.Value(u8).init(0);
+// 0 = uninitialized, 1 = initializing, 2 = initialized
/// Returns the per-process session ID as a UUID v4 string.
/// The value is generated lazily on first call and cached.
pub fn getSessionId() []const u8 {
- if (!session_id_initialized) {
+ // Double-checked locking with atomics
+ if (session_id_state.load(.acquire) == 2) {
+ return &session_id_buf;
+ }
+ // Try to claim initialization
+ if (session_id_state.cmpxchgStrong(0, 1, .acquire, .monotonic) == null) {
var bytes: [16]u8 = undefined;
std.crypto.random.bytes(&bytes);
// UUID v4: set version nibble to 4, variant bits to 10xx
bytes[6] = (bytes[6] & 0x0f) | 0x40;
bytes[8] = (bytes[8] & 0x3f) | 0x80;
formatUuidV4(&session_id_buf, bytes);
- session_id_initialized = true;
+ session_id_state.store(2, .release);
+ } else {
+ // Another thread is initializing; spin until done
+ while (session_id_state.load(.acquire) != 2) {
+ std.atomic.spinLoopHint();
+ }
}
return &session_id_buf;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var session_id_buf: [36]u8 = undefined; | |
| var session_id_initialized: bool = false; | |
| /// Returns the per-process session ID as a UUID v4 string. | |
| /// The value is generated lazily on first call and cached. | |
| pub fn getSessionId() []const u8 { | |
| if (!session_id_initialized) { | |
| var bytes: [16]u8 = undefined; | |
| std.crypto.random.bytes(&bytes); | |
| // UUID v4: set version nibble to 4, variant bits to 10xx | |
| bytes[6] = (bytes[6] & 0x0f) | 0x40; | |
| bytes[8] = (bytes[8] & 0x3f) | 0x80; | |
| formatUuidV4(&session_id_buf, bytes); | |
| session_id_initialized = true; | |
| } | |
| return &session_id_buf; | |
| } | |
| var session_id_buf: [36]u8 = undefined; | |
| var session_id_state: std.atomic.Value(u8) = std.atomic.Value(u8).init(0); | |
| // 0 = uninitialized, 1 = initializing, 2 = initialized | |
| /// Returns the per-process session ID as a UUID v4 string. | |
| /// The value is generated lazily on first call and cached. | |
| pub fn getSessionId() []const u8 { | |
| // Double-checked locking with atomics | |
| if (session_id_state.load(.acquire) == 2) { | |
| return &session_id_buf; | |
| } | |
| // Try to claim initialization | |
| if (session_id_state.cmpxchgStrong(0, 1, .acquire, .monotonic) == null) { | |
| var bytes: [16]u8 = undefined; | |
| std.crypto.random.bytes(&bytes); | |
| // UUID v4: set version nibble to 4, variant bits to 10xx | |
| bytes[6] = (bytes[6] & 0x0f) | 0x40; | |
| bytes[8] = (bytes[8] & 0x3f) | 0x80; | |
| formatUuidV4(&session_id_buf, bytes); | |
| session_id_state.store(2, .release); | |
| } else { | |
| // Another thread is initializing; spin until done | |
| while (session_id_state.load(.acquire) != 2) { | |
| std.atomic.spinLoopHint(); | |
| } | |
| } | |
| return &session_id_buf; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/docker_client.zig` around lines 57 - 73, The lazy initialization of
session_id_buf in getSessionId is not thread-safe; wrap the initialization with
a synchronization primitive so only one thread generates and writes the UUID and
others wait/see the initialized value. Replace the plain bool
session_id_initialized and unprotected write to session_id_buf with either a
mutex (std.Thread.Mutex) that locks around the generation and call to
formatUuidV4 inside getSessionId, or use an atomic flag (std.atomic.AtomicBool
or `@atomicCompareExchange` on session_id_initialized) to perform a
compare-and-swap "once" and ensure writers coordinate before writing
session_id_buf; update callers to return the buffer after the protected
initialization and ensure session_id_buf remains a stable global.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add standard testcontainers labels to all containers and networks
Summary
This PR implements the testcontainers label specification, automatically injecting four standard
org.testcontainers.*labels on every container and network created by the library. Previously only user-supplied labels were applied — no standard metadata was set.Motivation
org.testcontainers.*labels. This PR brings testcontainers-zig in line with that convention.org.testcontainers.sessionIdlabel enables future Ryuk-style reaper support to clean up orphaned containers.docker ps --filter label=org.testcontainersnow reliably identifies all testcontainers-managed resources.Standard labels injected
org.testcontainers"true"org.testcontainers.lang"zig"org.testcontainers.version"0.1.0"org.testcontainers.sessionIdWhat changed
label_base,label_lang,label_version,label_session_id, andtc_versionindocker_client.zigformatUuidV4and lazygetSessionId()— generates a per-process UUID on first call usingstd.crypto.randombuildCreateBodynow always injects the 4 standard labels before user labels (user labels take precedence)buildNetworkCreateBodynow always injects the same 4 standard labelstc_version, andgetSessionIdfromroot.zigCustomLabelsImageto verify all 4 standard labels on inspected containersKey new internal components
All located in
src/docker_client.zig:formatUuidV4— formats 16 random bytes as a RFC 4122 UUID v4 string (36 characters)getSessionId— lazily generates and caches a per-process UUID v4 session IDputDefaultLabels— writes the 4 standard labels into astd.json.ObjectMapDesign decisions
LabelsJSON object is emitted. This matches testcontainers-go behavior.std.fmt.fmtSliceHexLower— that API doesn't exist in Zig 0.15.2, so a manualformatUuidV4function handles hex encoding.Testing
zig build test --summary all— all 59 unit tests pass (no Docker required)zig build integration-test --summary all— integration suite validates labels on real containersSummary by Sourcery
Add standard Testcontainers metadata labels and a per-process session ID to all Docker containers and networks created by the library.
New Features:
Enhancements:
Tests:
Summary by CodeRabbit
Release Notes
New Features
Tests