Skip to content

Commit 3bb626e

Browse files
committed
Streams WriteResult.
Motivation: The stream writability API requires a poll after write (writeQueueFull) which can lead to a data race. A stream write could provide the writability as a result of the write operation, improving the implementation of writability poll. Changes: Introduce an internal WriteResult future, aimed to eventually replace stream writeQueueFull. This API is provided currently by HTTP server writes.
1 parent 074423f commit 3bb626e

6 files changed

Lines changed: 89 additions & 3 deletions

File tree

vertx-core/src/main/java/io/vertx/core/http/impl/http2/DefaultHttp2Stream.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,8 @@ public final boolean isWritable() {
269269
}
270270

271271
public final Future<Void> write(WritePromise write) {
272-
outboundQueue.write(write);
272+
boolean writable = outboundQueue.write(write);
273+
write.enqueued(writable);
273274
return write;
274275
}
275276

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package io.vertx.core.internal.streams;
2+
3+
import io.vertx.core.Future;
4+
5+
/**
6+
* Result of a write operation.
7+
*
8+
* @author <a href="mailto:julien@julienviet.com">Julien Viet</a>
9+
*/
10+
public interface WriteResult<T> extends Future<T> {
11+
12+
/**
13+
* @return whether the stream was writable after the write operation was submitted
14+
*/
15+
boolean isWritable();
16+
17+
}

vertx-core/src/main/java/io/vertx/core/net/impl/MessageWrite.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ public interface MessageWrite {
2121
*/
2222
void write();
2323

24+
default void enqueued(boolean writable) {
25+
}
26+
2427
/**
2528
* Cancel the write operation.
2629
*

vertx-core/src/main/java/io/vertx/core/net/impl/VertxConnection.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -481,7 +481,9 @@ public void cancel(Throwable cause) {
481481

482482
// Write to channel boolean return for now is not used so avoids reading a volatile
483483
public final boolean writeToChannel(MessageWrite msg) {
484-
return outboundMessageQueue.write(msg);
484+
boolean writable = outboundMessageQueue.write(msg);
485+
msg.enqueued(writable);
486+
return writable;
485487
}
486488

487489
/**

vertx-core/src/main/java/io/vertx/core/net/impl/WritePromise.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,32 @@
1212

1313
import io.vertx.core.impl.future.PromiseImpl;
1414
import io.vertx.core.internal.ContextInternal;
15+
import io.vertx.core.internal.streams.WriteResult;
1516

1617
/**
1718
* A write promise.
1819
*
1920
* @author <a href="mailto:julien@julienviet.com">Julien Viet</a>
2021
*/
21-
public abstract class WritePromise extends PromiseImpl<Void> implements MessageWrite {
22+
public abstract class WritePromise extends PromiseImpl<Void> implements MessageWrite, WriteResult<Void> {
23+
24+
private boolean writable;
2225

2326
public WritePromise(ContextInternal context) {
2427
super(context);
2528
}
2629

30+
@Override
31+
public void enqueued(boolean writable) {
32+
this.writable = writable;
33+
}
34+
2735
@Override
2836
public void cancel(Throwable cause) {
2937
fail(cause);
3038
}
39+
40+
public boolean isWritable() {
41+
return writable;
42+
}
3143
}

vertx-core/src/test/java/io/vertx/tests/http/HttpTest.java

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import io.vertx.core.internal.ContextInternal;
3333
import io.vertx.core.internal.http.HttpClientInternal;
3434
import io.vertx.core.internal.net.endpoint.EndpointResolverInternal;
35+
import io.vertx.core.internal.streams.WriteResult;
3536
import io.vertx.core.json.JsonObject;
3637
import io.vertx.core.net.*;
3738
import io.vertx.core.net.impl.MessageWrite;
@@ -6686,4 +6687,54 @@ public void testServerMessageWrite() throws Exception {
66866687
assertTrue(ref1.get() instanceof MessageWrite);
66876688
assertTrue(ref2.get() instanceof MessageWrite);
66886689
}
6690+
6691+
private void stableUnwritable(Buffer chunk, HttpServerResponse response, WriteResult<?> prev, Consumer<WriteResult<?>> done) {
6692+
WriteResult<?> last = null;
6693+
while (true) {
6694+
if (response.writeQueueFull()) {
6695+
WriteResult<?> w = last;
6696+
if (w == null) {
6697+
// Done => use prev
6698+
done.accept(prev);
6699+
} else {
6700+
// Give some time to fill the window
6701+
vertx.setTimer(10, v -> {
6702+
stableUnwritable(chunk, response, w, done);
6703+
});
6704+
}
6705+
break;
6706+
} else {
6707+
last = (WriteResult<?>)response.write(chunk);
6708+
}
6709+
}
6710+
}
6711+
6712+
@Test
6713+
public void testServerMessageWriteWritability(Checkpoint checkpoint1, Checkpoint checkpoint2, Checkpoint checkpoint3) throws Exception {
6714+
server.requestHandler(request -> {
6715+
HttpServerResponse response = request.response();
6716+
response.setChunked(true);
6717+
vertx.runOnContext(v1 -> {
6718+
Buffer chunk = Buffer.buffer(TestUtils.randomAlphaString(1024));
6719+
stableUnwritable(chunk, response, null, last -> {
6720+
assertEquals(last.isWritable(), !response.writeQueueFull());
6721+
last.onComplete(v -> {
6722+
checkpoint2.succeed();
6723+
});
6724+
response.drainHandler(v2 -> {
6725+
checkpoint3.succeed();
6726+
});
6727+
checkpoint1.succeed();
6728+
});
6729+
});
6730+
});
6731+
startServer();
6732+
HttpClientResponse response = client.request(new RequestOptions(requestOptions).setPort(server.actualPort()))
6733+
.compose(req -> req
6734+
.send()
6735+
.expecting(HttpResponseExpectation.SC_OK)
6736+
.andThen(onSuccess(HttpClientResponse::pause))).await();
6737+
checkpoint1.awaitSuccess();
6738+
response.resume();
6739+
}
66896740
}

0 commit comments

Comments
 (0)