Skip to content

Commit 714d30b

Browse files
original-brownbearjfreden
authored andcommitted
Unlink delegate in ReleasableBytesReference once ref count reaches 0 (elastic#127058)
We have some spots where we retain a reference to the `ReleasableBytesReference` instance well beyond its ref-count reaching `0`. If it itself references Netty buffers or `BigArrays` that are not pooled (mostly as a result of overflowing the pooled number of bytes for large messages or under heavy load) then those bytes are not GC-able unless we unlink them here.
1 parent c7f2a9a commit 714d30b

File tree

2 files changed

+17
-6
lines changed

2 files changed

+17
-6
lines changed

server/src/main/java/org/elasticsearch/common/bytes/ReleasableBytesReference.java

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public final class ReleasableBytesReference implements RefCounted, Releasable, B
2929

3030
private static final ReleasableBytesReference EMPTY = new ReleasableBytesReference(BytesArray.EMPTY, RefCounted.ALWAYS_REFERENCED);
3131

32-
private final BytesReference delegate;
32+
private BytesReference delegate;
3333
private final RefCounted refCounted;
3434

3535
public static ReleasableBytesReference empty() {
@@ -63,16 +63,24 @@ public boolean tryIncRef() {
6363

6464
@Override
6565
public boolean decRef() {
66-
return refCounted.decRef();
66+
boolean res = refCounted.decRef();
67+
if (res) {
68+
delegate = null;
69+
}
70+
return res;
6771
}
6872

6973
@Override
7074
public boolean hasReferences() {
71-
return refCounted.hasReferences();
75+
boolean hasRef = refCounted.hasReferences();
76+
// delegate is nulled out when the ref-count reaches zero but only via a plain store, and also we could be racing with a concurrent
77+
// decRef so need to check #refCounted again in case we run into a non-null delegate but saw a reference before
78+
assert delegate != null || refCounted.hasReferences() == false;
79+
return hasRef;
7280
}
7381

7482
public ReleasableBytesReference retain() {
75-
refCounted.incRef();
83+
refCounted.mustIncRef();
7684
return this;
7785
}
7886

@@ -82,6 +90,7 @@ public ReleasableBytesReference retain() {
8290
* retaining unnecessary buffers.
8391
*/
8492
public ReleasableBytesReference retainedSlice(int from, int length) {
93+
assert hasReferences();
8594
if (from == 0 && length() == length) {
8695
return retain();
8796
}
@@ -136,6 +145,7 @@ public int indexOf(byte marker, int from) {
136145

137146
@Override
138147
public int length() {
148+
assert hasReferences();
139149
return delegate.length();
140150
}
141151

@@ -154,6 +164,7 @@ public ReleasableBytesReference slice(int from, int length) {
154164

155165
@Override
156166
public long ramBytesUsed() {
167+
assert hasReferences();
157168
return delegate.ramBytesUsed();
158169
}
159170

@@ -233,6 +244,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
233244

234245
@Override
235246
public boolean isFragment() {
247+
assert hasReferences();
236248
return delegate.isFragment();
237249
}
238250

x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityFcActionAuthorizationIT.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@
8080
import static org.elasticsearch.xpack.remotecluster.AbstractRemoteClusterSecurityTestCase.performRequestWithAdminUser;
8181
import static org.hamcrest.Matchers.arrayContaining;
8282
import static org.hamcrest.Matchers.containsString;
83-
import static org.hamcrest.Matchers.equalTo;
8483
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
8584
import static org.hamcrest.Matchers.hasSize;
8685
import static org.hamcrest.Matchers.instanceOf;
@@ -282,7 +281,7 @@ public void testIndicesPrivilegesAreEnforcedForCcrRestoreSessionActions() throws
282281
GetCcrRestoreFileChunkAction.REMOTE_TYPE,
283282
new GetCcrRestoreFileChunkRequest(response2.getNode(), sessionUUID2, leaderIndex2FileName, 1, shardId2)
284283
);
285-
assertThat(getChunkResponse.getChunk().length(), equalTo(1));
284+
assertFalse(getChunkResponse.getChunk().hasReferences());
286285

287286
// Clear restore session fails if index is unauthorized
288287
final var e4 = expectThrows(

0 commit comments

Comments
 (0)