Skip to content

Commit 6e4ac2b

Browse files
Add Ref Count Assertion to Page (elastic#69599)
Added this assertion to have an easier time debugging work on elastic#67502 and found that we were accessing `refcount == 0` bytes in the `SSLOutboundBuffer` so I fixed that buffer to not keep references to released pages.
1 parent a5f6c1b commit 6e4ac2b

File tree

2 files changed

+32
-13
lines changed
  • libs/nio/src/main/java/org/elasticsearch/nio
  • x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio

2 files changed

+32
-13
lines changed

libs/nio/src/main/java/org/elasticsearch/nio/Page.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,12 @@ public class Page implements Closeable {
2222
// released.
2323
private final RefCountedCloseable refCountedCloseable;
2424

25-
public Page(ByteBuffer byteBuffer) {
26-
this(byteBuffer, () -> {});
27-
}
28-
2925
public Page(ByteBuffer byteBuffer, Runnable closeable) {
3026
this(byteBuffer, new RefCountedCloseable(closeable));
3127
}
3228

3329
private Page(ByteBuffer byteBuffer, RefCountedCloseable refCountedCloseable) {
30+
assert refCountedCloseable.refCount() > 0;
3431
this.byteBuffer = byteBuffer;
3532
this.refCountedCloseable = refCountedCloseable;
3633
}
@@ -53,6 +50,7 @@ public Page duplicate() {
5350
* @return the byte buffer
5451
*/
5552
public ByteBuffer byteBuffer() {
53+
assert refCountedCloseable.refCount() > 0;
5654
return byteBuffer;
5755
}
5856

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SSLOutboundBuffer.java

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@
66
*/
77
package org.elasticsearch.xpack.security.transport.nio;
88

9+
import org.elasticsearch.ElasticsearchException;
10+
import org.elasticsearch.ExceptionsHelper;
911
import org.elasticsearch.core.internal.io.IOUtils;
1012
import org.elasticsearch.nio.FlushOperation;
1113
import org.elasticsearch.nio.Page;
1214

1315
import java.nio.ByteBuffer;
1416
import java.util.ArrayDeque;
15-
import java.util.function.BiConsumer;
1617
import java.util.function.IntFunction;
1718

1819
public class SSLOutboundBuffer implements AutoCloseable {
@@ -49,10 +50,6 @@ ByteBuffer nextWriteBuffer(int networkBufferSize) {
4950
}
5051

5152
FlushOperation buildNetworkFlushOperation() {
52-
return buildNetworkFlushOperation((r, e) -> {});
53-
}
54-
55-
FlushOperation buildNetworkFlushOperation(BiConsumer<Void, Exception> listener) {
5653
int pageCount = pages.size();
5754
ByteBuffer[] byteBuffers = new ByteBuffer[pageCount];
5855
Page[] pagesToClose = new Page[pageCount];
@@ -63,8 +60,15 @@ FlushOperation buildNetworkFlushOperation(BiConsumer<Void, Exception> listener)
6360
}
6461

6562
return new FlushOperation(byteBuffers, (r, e) -> {
66-
IOUtils.closeWhileHandlingException(pagesToClose);
67-
listener.accept(r, e);
63+
try {
64+
IOUtils.close(pagesToClose);
65+
} catch (Exception ex) {
66+
if (e != null) {
67+
ex.addSuppressed(e);
68+
}
69+
assert false : ex;
70+
throw new ElasticsearchException(ex);
71+
}
6872
});
6973
}
7074

@@ -74,7 +78,24 @@ boolean hasEncryptedBytesToFlush() {
7478

7579
@Override
7680
public void close() {
77-
IOUtils.closeWhileHandlingException(currentPage);
78-
IOUtils.closeWhileHandlingException(pages);
81+
Exception closeException = null;
82+
try {
83+
IOUtils.close(currentPage);
84+
} catch (Exception e) {
85+
closeException = e;
86+
}
87+
currentPage = null;
88+
Page p;
89+
while ((p = pages.pollFirst()) != null) {
90+
try {
91+
p.close();
92+
} catch (Exception ex) {
93+
closeException = ExceptionsHelper.useOrSuppress(closeException, ex);
94+
}
95+
}
96+
if (closeException != null) {
97+
assert false : closeException;
98+
throw new ElasticsearchException(closeException);
99+
}
79100
}
80101
}

0 commit comments

Comments
 (0)