Skip to content

Commit 2f76dd4

Browse files
committed
feat(profiling): Reference-count RecordingData lifecycle
RecordingData gains explicit retain/release semantics so multiple consumers (recording streaming and the OTLP uploader) can share a recording without one side closing it prematurely. Implemented across the OpenJDK, OracleJDK and ddprof controller backends.
1 parent f36694b commit 2f76dd4

5 files changed

Lines changed: 307 additions & 14 deletions

File tree

‎dd-java-agent/agent-profiling/profiling-controller-openjdk/src/main/java/com/datadog/profiling/controller/openjdk/OpenJdkRecordingData.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public RecordingInputStream getStream() throws IOException {
4545
}
4646

4747
@Override
48-
public void release() {
48+
protected void doRelease() {
4949
recording.close();
5050
}
5151

‎dd-java-agent/agent-profiling/profiling-controller-oracle/src/main/java/com/datadog/profiling/controller/oracle/OracleJdkRecordingData.java‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,13 @@
2323
import java.util.Date;
2424
import javax.annotation.Nonnull;
2525
import javax.management.ObjectName;
26+
import org.slf4j.Logger;
27+
import org.slf4j.LoggerFactory;
2628

2729
/** Implementation for profiling recordings. */
2830
public class OracleJdkRecordingData extends RecordingData {
31+
private static final Logger log = LoggerFactory.getLogger(OracleJdkRecordingData.class);
32+
2933
private final ObjectName recordingId;
3034
private final String name;
3135

@@ -51,8 +55,15 @@ public RecordingInputStream getStream() throws IOException {
5155
}
5256

5357
@Override
54-
public void release() {
55-
// noop
58+
protected void doRelease() {
59+
// the recording is shared by every listener's stream — the OTLP listener may close its
60+
// stream while the classic uploader still needs to open its own, so the recording itself
61+
// can only be closed when the last reference is released
62+
try {
63+
helper.closeRecording(recordingId);
64+
} catch (Exception e) {
65+
log.warn("Failed to close Oracle JDK recording {}", recordingId, e);
66+
}
5667
}
5768

5869
@Override
@@ -116,7 +127,8 @@ public void close() throws IOException {
116127
if (streamId != -1) {
117128
helper.closeStream(streamId);
118129
}
119-
helper.closeRecording(recordingId);
130+
// the recording itself is closed in doRelease() — closing it here would prevent any
131+
// other listener from opening its own stream on the same recording
120132
} catch (Exception e) {
121133
throw new IOException(e);
122134
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,223 @@
1+
/*
2+
* Copyright 2026 Datadog
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.datadog.profiling.controller;
17+
18+
import static org.junit.jupiter.api.Assertions.assertEquals;
19+
import static org.junit.jupiter.api.Assertions.assertThrows;
20+
import static org.junit.jupiter.api.Assertions.assertTrue;
21+
22+
import datadog.trace.api.profiling.ProfilingSnapshot;
23+
import datadog.trace.api.profiling.RecordingData;
24+
import datadog.trace.api.profiling.RecordingInputStream;
25+
import java.io.ByteArrayInputStream;
26+
import java.io.IOException;
27+
import java.nio.file.Path;
28+
import java.time.Instant;
29+
import java.util.concurrent.CountDownLatch;
30+
import java.util.concurrent.TimeUnit;
31+
import java.util.concurrent.atomic.AtomicInteger;
32+
import javax.annotation.Nonnull;
33+
import javax.annotation.Nullable;
34+
import org.junit.jupiter.api.Test;
35+
36+
/**
37+
* Tests for RecordingData reference counting with multiple handlers. The reference count starts at
38+
* 1 (the base reference). The primary listener releases the base reference; each additional
39+
* listener calls retain() then release().
40+
*/
41+
public class RecordingDataRefCountingTest {
42+
43+
/** Test RecordingData implementation that tracks release calls. */
44+
private static class TestRecordingData extends RecordingData {
45+
private final AtomicInteger releaseCount = new AtomicInteger(0);
46+
private final CountDownLatch releaseLatch = new CountDownLatch(1);
47+
48+
public TestRecordingData() {
49+
super(Instant.now(), Instant.now(), ProfilingSnapshot.Kind.PERIODIC);
50+
}
51+
52+
@Nonnull
53+
@Override
54+
public RecordingInputStream getStream() throws IOException {
55+
return new RecordingInputStream(new ByteArrayInputStream(new byte[0]));
56+
}
57+
58+
@Override
59+
protected void doRelease() {
60+
releaseCount.incrementAndGet();
61+
releaseLatch.countDown();
62+
}
63+
64+
@Nullable
65+
@Override
66+
public Path getPath() {
67+
return null;
68+
}
69+
70+
@Override
71+
public String getName() {
72+
return "test-recording";
73+
}
74+
75+
public int getReleaseCount() {
76+
return releaseCount.get();
77+
}
78+
79+
public boolean awaitRelease(long timeout, TimeUnit unit) throws InterruptedException {
80+
return releaseLatch.await(timeout, unit);
81+
}
82+
}
83+
84+
@Test
85+
public void testSingleHandler() throws InterruptedException {
86+
TestRecordingData data = new TestRecordingData();
87+
88+
// Single handler: just release the base reference (no retain needed)
89+
data.release();
90+
91+
assertTrue(data.awaitRelease(1, TimeUnit.SECONDS), "Release should be called");
92+
assertEquals(1, data.getReleaseCount(), "doRelease() should be called exactly once");
93+
}
94+
95+
@Test
96+
public void testTwoHandlers() throws InterruptedException {
97+
TestRecordingData data = new TestRecordingData();
98+
99+
// Two handlers: retain once for the additional handler, release twice
100+
data.retain(); // Additional handler (e.g., OTLP)
101+
assertEquals(0, data.getReleaseCount(), "Should not be released yet");
102+
103+
// First handler releases (base reference)
104+
data.release();
105+
assertEquals(0, data.getReleaseCount(), "Should not be released after first release");
106+
107+
// Second handler releases
108+
data.release();
109+
110+
assertTrue(data.awaitRelease(1, TimeUnit.SECONDS), "Release should be called");
111+
assertEquals(1, data.getReleaseCount(), "doRelease() should be called exactly once");
112+
}
113+
114+
@Test
115+
public void testThreeHandlers() throws InterruptedException {
116+
TestRecordingData data = new TestRecordingData();
117+
118+
// Three handlers: retain twice, release three times
119+
data.retain(); // Additional handler 1
120+
data.retain(); // Additional handler 2
121+
assertEquals(0, data.getReleaseCount(), "Should not be released yet");
122+
123+
// First two handlers release
124+
data.release();
125+
data.release();
126+
assertEquals(0, data.getReleaseCount(), "Should not be released after two releases");
127+
128+
// Third handler releases
129+
data.release();
130+
131+
assertTrue(data.awaitRelease(1, TimeUnit.SECONDS), "Release should be called");
132+
assertEquals(1, data.getReleaseCount(), "doRelease() should be called exactly once");
133+
}
134+
135+
@Test
136+
public void testOverReleaseIsSilentNoOp() throws InterruptedException {
137+
TestRecordingData data = new TestRecordingData();
138+
139+
// Release the base reference
140+
data.release();
141+
assertTrue(data.awaitRelease(1, TimeUnit.SECONDS), "Release should be called");
142+
143+
// Additional release after full release is a silent no-op — unlike retain() after full release
144+
data.release();
145+
assertEquals(1, data.getReleaseCount(), "doRelease() should still be called exactly once");
146+
}
147+
148+
@Test
149+
public void testRetainAfterFullRelease() throws InterruptedException {
150+
TestRecordingData data = new TestRecordingData();
151+
152+
data.release();
153+
assertTrue(data.awaitRelease(1, TimeUnit.SECONDS), "Release should be called");
154+
155+
// Cannot retain after full release
156+
assertThrows(
157+
IllegalStateException.class, data::retain, "Should throw when retaining after release");
158+
}
159+
160+
@Test
161+
public void testMultipleReleaseIdempotent() throws InterruptedException {
162+
TestRecordingData data = new TestRecordingData();
163+
164+
data.release();
165+
assertTrue(data.awaitRelease(1, TimeUnit.SECONDS), "Release should be called");
166+
167+
// Additional release calls should be silent no-ops (released flag guards them)
168+
data.release();
169+
data.release();
170+
171+
assertEquals(1, data.getReleaseCount(), "doRelease() should still be called exactly once");
172+
}
173+
174+
@Test
175+
public void testConcurrentHandlers() throws InterruptedException {
176+
TestRecordingData data = new TestRecordingData();
177+
int numAdditionalHandlers = 9; // total 10 with the base reference
178+
179+
// Retain for all additional handlers
180+
for (int i = 0; i < numAdditionalHandlers; i++) {
181+
data.retain();
182+
}
183+
184+
int totalHandlers = numAdditionalHandlers + 1;
185+
186+
// Simulate concurrent release from multiple threads
187+
CountDownLatch startLatch = new CountDownLatch(1);
188+
CountDownLatch doneLatch = new CountDownLatch(totalHandlers);
189+
190+
for (int i = 0; i < totalHandlers; i++) {
191+
new Thread(
192+
() -> {
193+
try {
194+
startLatch.await();
195+
data.release();
196+
} catch (InterruptedException e) {
197+
Thread.currentThread().interrupt();
198+
} finally {
199+
// count down even on interrupt, otherwise it masks as a liveness timeout below
200+
doneLatch.countDown();
201+
}
202+
})
203+
.start();
204+
}
205+
206+
// Start all threads
207+
startLatch.countDown();
208+
209+
// Wait for all threads to complete
210+
assertTrue(doneLatch.await(5, TimeUnit.SECONDS), "All threads should complete");
211+
assertTrue(data.awaitRelease(1, TimeUnit.SECONDS), "Release should be called");
212+
assertEquals(1, data.getReleaseCount(), "doRelease() should be called exactly once");
213+
}
214+
215+
@Test
216+
public void testRetainChaining() {
217+
TestRecordingData data = new TestRecordingData();
218+
219+
// retain() should return this for chaining
220+
RecordingData result = data.retain();
221+
assertEquals(data, result, "retain() should return the same instance");
222+
}
223+
}

‎dd-java-agent/agent-profiling/profiling-ddprof/src/main/java/com/datadog/profiling/ddprof/DatadogProfilerRecordingData.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public RecordingInputStream getStream() throws IOException {
2424
}
2525

2626
@Override
27-
public void release() {
27+
protected void doRelease() {
2828
try {
2929
Files.deleteIfExists(recordingFile);
3030
} catch (IOException e) {

‎internal-api/src/main/java/datadog/trace/api/profiling/RecordingData.java‎

Lines changed: 67 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,29 @@
2121
import javax.annotation.Nonnull;
2222
import javax.annotation.Nullable;
2323

24-
/** Platform-agnostic API for operations required when retrieving data using the ProfilingSystem. */
24+
/**
25+
* Platform-agnostic API for operations required when retrieving data using the ProfilingSystem.
26+
*
27+
* <p>Subclassing note: {@link #release()} is final and reference-counted; subclasses implement
28+
* resource cleanup by overriding {@link #doRelease()}, invoked exactly once when the reference
29+
* count reaches zero.
30+
*/
2531
public abstract class RecordingData implements ProfilingSnapshot {
2632
protected final Instant start;
2733
protected final Instant end;
2834
protected final Kind kind;
2935

36+
// Reference counting for multiple listeners. Starts at 1 (the base reference owned by the
37+
// first listener in the chain). Additional listeners call retain() before use and release()
38+
// when done. doRelease() fires when the count reaches 0.
39+
// Plain fields suffice: both are only ever accessed under the private lock below.
40+
private int refCount = 1;
41+
private boolean released = false;
42+
43+
// Private lock so the intrinsic lock of this publicly exposed class is not used for
44+
// synchronization (SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION / CERT LCK00-J).
45+
private final Object lock = new Object();
46+
3047
public RecordingData(final Instant start, final Instant end, Kind kind) {
3148
this.start = start;
3249
this.end = end;
@@ -41,18 +58,59 @@ public RecordingData(final Instant start, final Instant end, Kind kind) {
4158
public abstract RecordingInputStream getStream() throws IOException;
4259

4360
/**
44-
* Releases the resources associated with the recording, for example the underlying file.
61+
* Must be called once for each handler beyond the first, before it processes the recording. The
62+
* counting scheme is documented on the {@code refCount} field.
63+
*
64+
* @return this instance for chaining
65+
* @throws IllegalStateException if the recording has already been released
66+
*/
67+
@Nonnull
68+
public final RecordingData retain() {
69+
synchronized (lock) {
70+
if (released) {
71+
throw new IllegalStateException("Cannot retain released RecordingData");
72+
}
73+
refCount++;
74+
return this;
75+
}
76+
}
77+
78+
/**
79+
* Releases one reference to the recording. The resources are released when the reference count
80+
* reaches zero (see the {@code refCount} field for the counting scheme).
4581
*
46-
* <p>Forgetting to releasing this when done streaming, will lead to one or more of the following:
82+
* <p>Forgetting to release when done streaming leaks the recording file and memory.
4783
*
48-
* <ul>
49-
* <li>Memory leak
50-
* <li>File leak
51-
* </ul>
84+
* <p>Releasing an already fully-released recording is a silent no-op: the {@code released} flag
85+
* guards against double release. Retaining after full release, in contrast, throws {@link
86+
* IllegalStateException}.
87+
*/
88+
public final void release() {
89+
boolean shouldRelease = false;
90+
synchronized (lock) {
91+
if (released) {
92+
// retain() refuses to hand out new references once released, so the count can never
93+
// reach zero a second time
94+
return;
95+
}
96+
int remaining = --refCount;
97+
if (remaining == 0) {
98+
released = true;
99+
shouldRelease = true;
100+
}
101+
}
102+
if (shouldRelease) {
103+
doRelease();
104+
}
105+
}
106+
107+
/**
108+
* Actual resource cleanup implementation. Subclasses must override this method instead of {@link
109+
* #release()}.
52110
*
53-
* <p>Please don't forget to call release when done streaming...
111+
* <p>This method is called exactly once when the reference count reaches zero.
54112
*/
55-
public abstract void release();
113+
protected abstract void doRelease();
56114

57115
/**
58116
* Returns the name of the recording from which the data is originating.

0 commit comments

Comments
 (0)