Merge branch '5.1.x'

This commit is contained in:
Rossen Stoyanchev
2019-03-26 21:14:51 -04:00
21 changed files with 538 additions and 218 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@ import reactor.core.publisher.Flux;
import org.springframework.core.ResolvableType;
import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.core.io.buffer.DataBufferFactory;
import org.springframework.core.io.buffer.PooledDataBuffer;
import org.springframework.lang.Nullable;
import org.springframework.util.MimeType;
@@ -47,9 +48,10 @@ public abstract class AbstractSingleValueEncoder<T> extends AbstractEncoder<T> {
public final Flux<DataBuffer> encode(Publisher<? extends T> inputStream, DataBufferFactory bufferFactory,
ResolvableType elementType, @Nullable MimeType mimeType, @Nullable Map<String, Object> hints) {
return Flux.from(inputStream).
take(1).
concatMap(t -> encode(t, bufferFactory, elementType, mimeType, hints));
return Flux.from(inputStream)
.take(1)
.concatMap(value -> encode(value, bufferFactory, elementType, mimeType, hints))
.doOnDiscard(PooledDataBuffer.class, PooledDataBuffer::release);
}
/**

View File

@@ -17,7 +17,6 @@
package org.springframework.core.codec;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.Map;
import java.util.OptionalLong;
@@ -89,24 +88,22 @@ public class ResourceRegionEncoder extends AbstractEncoder<ResourceRegion> {
return Mono.from(inputStream)
.flatMapMany(region -> {
if (!region.getResource().isReadable()) {
return Flux.error(new EncodingException("Resource " +
region.getResource() + " is not readable"));
return Flux.error(new EncodingException(
"Resource " + region.getResource() + " is not readable"));
}
return writeResourceRegion(region, bufferFactory, hints);
});
}
else {
final String boundaryString = Hints.getRequiredHint(hints, BOUNDARY_STRING_HINT);
byte[] startBoundary = getAsciiBytes("\r\n--" + boundaryString + "\r\n");
byte[] contentType =
(mimeType != null ? getAsciiBytes("Content-Type: " + mimeType + "\r\n") : new byte[0]);
byte[] contentType = mimeType != null ? getAsciiBytes("Content-Type: " + mimeType + "\r\n") : new byte[0];
return Flux.from(inputStream).
concatMap(region -> {
if (!region.getResource().isReadable()) {
return Flux.error(new EncodingException("Resource " +
region.getResource() + " is not readable"));
return Flux.error(new EncodingException(
"Resource " + region.getResource() + " is not readable"));
}
else {
return Flux.concat(
@@ -121,11 +118,10 @@ public class ResourceRegionEncoder extends AbstractEncoder<ResourceRegion> {
private Flux<DataBuffer> getRegionPrefix(DataBufferFactory bufferFactory, byte[] startBoundary,
byte[] contentType, ResourceRegion region) {
return Flux.defer(() -> Flux.just(
bufferFactory.allocateBuffer(startBoundary.length).write(startBoundary),
bufferFactory.allocateBuffer(contentType.length).write(contentType),
bufferFactory.wrap(ByteBuffer.wrap(getContentRangeHeader(region))))
);
return Flux.just(
bufferFactory.wrap(startBoundary),
bufferFactory.wrap(contentType),
bufferFactory.wrap(getContentRangeHeader(region))); // only wrapping, no allocation
}
private Flux<DataBuffer> writeResourceRegion(
@@ -146,8 +142,7 @@ public class ResourceRegionEncoder extends AbstractEncoder<ResourceRegion> {
private Flux<DataBuffer> getRegionSuffix(DataBufferFactory bufferFactory, String boundaryString) {
byte[] endBoundary = getAsciiBytes("\r\n--" + boundaryString + "--");
return Flux.defer(() -> Flux.just(
bufferFactory.allocateBuffer(endBoundary.length).write(endBoundary)));
return Flux.just(bufferFactory.wrap(endBoundary));
}
private byte[] getAsciiBytes(String in) {

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -82,40 +82,36 @@ public abstract class DataBufferUtils {
* Obtain a {@link ReadableByteChannel} from the given supplier, and read it into a
* {@code Flux} of {@code DataBuffer}s. Closes the channel when the flux is terminated.
* @param channelSupplier the supplier for the channel to read from
* @param dataBufferFactory the factory to create data buffers with
* @param bufferFactory the factory to create data buffers with
* @param bufferSize the maximum size of the data buffers
* @return a flux of data buffers read from the given channel
*/
public static Flux<DataBuffer> readByteChannel(
Callable<ReadableByteChannel> channelSupplier, DataBufferFactory dataBufferFactory, int bufferSize) {
Callable<ReadableByteChannel> channelSupplier, DataBufferFactory bufferFactory, int bufferSize) {
Assert.notNull(channelSupplier, "'channelSupplier' must not be null");
Assert.notNull(dataBufferFactory, "'dataBufferFactory' must not be null");
Assert.notNull(bufferFactory, "'dataBufferFactory' must not be null");
Assert.isTrue(bufferSize > 0, "'bufferSize' must be > 0");
return Flux.using(channelSupplier,
channel -> {
ReadableByteChannelGenerator generator =
new ReadableByteChannelGenerator(channel, dataBufferFactory,
bufferSize);
return Flux.generate(generator);
},
DataBufferUtils::closeChannel)
.doOnDiscard(PooledDataBuffer.class, DataBufferUtils::release);
channel -> Flux.generate(new ReadableByteChannelGenerator(channel, bufferFactory, bufferSize)),
DataBufferUtils::closeChannel);
// No doOnDiscard as operators used do not cache
}
/**
* Obtain a {@code AsynchronousFileChannel} from the given supplier, and read it into a
* {@code Flux} of {@code DataBuffer}s. Closes the channel when the flux is terminated.
* @param channelSupplier the supplier for the channel to read from
* @param dataBufferFactory the factory to create data buffers with
* @param bufferFactory the factory to create data buffers with
* @param bufferSize the maximum size of the data buffers
* @return a flux of data buffers read from the given channel
*/
public static Flux<DataBuffer> readAsynchronousFileChannel(
Callable<AsynchronousFileChannel> channelSupplier, DataBufferFactory dataBufferFactory, int bufferSize) {
Callable<AsynchronousFileChannel> channelSupplier, DataBufferFactory bufferFactory, int bufferSize) {
return readAsynchronousFileChannel(channelSupplier, 0, dataBufferFactory, bufferSize);
return readAsynchronousFileChannel(channelSupplier, 0, bufferFactory, bufferSize);
}
/**
@@ -124,32 +120,30 @@ public abstract class DataBufferUtils {
* channel when the flux is terminated.
* @param channelSupplier the supplier for the channel to read from
* @param position the position to start reading from
* @param dataBufferFactory the factory to create data buffers with
* @param bufferFactory the factory to create data buffers with
* @param bufferSize the maximum size of the data buffers
* @return a flux of data buffers read from the given channel
*/
public static Flux<DataBuffer> readAsynchronousFileChannel(Callable<AsynchronousFileChannel> channelSupplier,
long position, DataBufferFactory dataBufferFactory, int bufferSize) {
long position, DataBufferFactory bufferFactory, int bufferSize) {
Assert.notNull(channelSupplier, "'channelSupplier' must not be null");
Assert.notNull(dataBufferFactory, "'dataBufferFactory' must not be null");
Assert.notNull(bufferFactory, "'dataBufferFactory' must not be null");
Assert.isTrue(position >= 0, "'position' must be >= 0");
Assert.isTrue(bufferSize > 0, "'bufferSize' must be > 0");
DataBuffer dataBuffer = dataBufferFactory.allocateBuffer(bufferSize);
ByteBuffer byteBuffer = dataBuffer.asByteBuffer(0, bufferSize);
Flux<DataBuffer> result = Flux.using(channelSupplier,
Flux<DataBuffer> flux = Flux.using(channelSupplier,
channel -> Flux.create(sink -> {
AsynchronousFileChannelReadCompletionHandler completionHandler =
new AsynchronousFileChannelReadCompletionHandler(channel,
sink, position, dataBufferFactory, bufferSize);
channel.read(byteBuffer, position, dataBuffer, completionHandler);
sink.onDispose(completionHandler::dispose);
ReadCompletionHandler handler =
new ReadCompletionHandler(channel, sink, position, bufferFactory, bufferSize);
DataBuffer dataBuffer = bufferFactory.allocateBuffer(bufferSize);
ByteBuffer byteBuffer = dataBuffer.asByteBuffer(0, bufferSize);
channel.read(byteBuffer, position, dataBuffer, handler);
sink.onDispose(handler::dispose);
}),
DataBufferUtils::closeChannel);
return result.doOnDiscard(PooledDataBuffer.class, DataBufferUtils::release);
return flux.doOnDiscard(PooledDataBuffer.class, DataBufferUtils::release);
}
/**
@@ -246,8 +240,7 @@ public abstract class DataBufferUtils {
Flux<DataBuffer> flux = Flux.from(source);
return Flux.create(sink -> {
WritableByteChannelSubscriber subscriber =
new WritableByteChannelSubscriber(sink, channel);
WritableByteChannelSubscriber subscriber = new WritableByteChannelSubscriber(sink, channel);
sink.onDispose(subscriber);
flux.subscribe(subscriber);
});
@@ -292,10 +285,9 @@ public abstract class DataBufferUtils {
Flux<DataBuffer> flux = Flux.from(source);
return Flux.create(sink -> {
AsynchronousFileChannelWriteCompletionHandler completionHandler =
new AsynchronousFileChannelWriteCompletionHandler(sink, channel, position);
sink.onDispose(completionHandler);
flux.subscribe(completionHandler);
WriteCompletionHandler handler = new WriteCompletionHandler(sink, channel, position);
sink.onDispose(handler);
flux.subscribe(handler);
});
}
@@ -326,21 +318,21 @@ public abstract class DataBufferUtils {
Assert.notNull(publisher, "Publisher must not be null");
Assert.isTrue(maxByteCount >= 0, "'maxByteCount' must be a positive number");
return Flux.defer(() -> {
AtomicLong countDown = new AtomicLong(maxByteCount);
return Flux.from(publisher)
.map(buffer -> {
long remainder = countDown.addAndGet(-buffer.readableByteCount());
if (remainder < 0) {
int length = buffer.readableByteCount() + (int) remainder;
return buffer.slice(0, length);
}
else {
return buffer;
}
})
.takeUntil(buffer -> countDown.get() <= 0);
}); // no doOnDiscard necessary, as this method does not drop buffers
AtomicLong countDown = new AtomicLong(maxByteCount);
return Flux.from(publisher)
.map(buffer -> {
long remainder = countDown.addAndGet(-buffer.readableByteCount());
if (remainder < 0) {
int length = buffer.readableByteCount() + (int) remainder;
return buffer.slice(0, length);
}
else {
return buffer;
}
})
.takeUntil(buffer -> countDown.get() <= 0);
// No doOnDiscard as operators used do not cache (and drop) buffers
}
/**
@@ -487,8 +479,7 @@ public abstract class DataBufferUtils {
}
private static class AsynchronousFileChannelReadCompletionHandler
implements CompletionHandler<Integer, DataBuffer> {
private static class ReadCompletionHandler implements CompletionHandler<Integer, DataBuffer> {
private final AsynchronousFileChannel channel;
@@ -502,7 +493,7 @@ public abstract class DataBufferUtils {
private final AtomicBoolean disposed = new AtomicBoolean();
public AsynchronousFileChannelReadCompletionHandler(AsynchronousFileChannel channel,
public ReadCompletionHandler(AsynchronousFileChannel channel,
FluxSink<DataBuffer> sink, long position, DataBufferFactory dataBufferFactory, int bufferSize) {
this.channel = channel;
@@ -586,7 +577,7 @@ public abstract class DataBufferUtils {
}
private static class AsynchronousFileChannelWriteCompletionHandler extends BaseSubscriber<DataBuffer>
private static class WriteCompletionHandler extends BaseSubscriber<DataBuffer>
implements CompletionHandler<Integer, ByteBuffer> {
private final FluxSink<DataBuffer> sink;
@@ -601,7 +592,7 @@ public abstract class DataBufferUtils {
private final AtomicReference<DataBuffer> dataBuffer = new AtomicReference<>();
public AsynchronousFileChannelWriteCompletionHandler(
public WriteCompletionHandler(
FluxSink<DataBuffer> sink, AsynchronousFileChannel channel, long position) {
this.sink = sink;

View File

@@ -20,6 +20,8 @@ import java.util.Collections;
import java.util.function.Consumer;
import org.junit.Test;
import org.reactivestreams.Subscription;
import reactor.core.publisher.BaseSubscriber;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
@@ -28,7 +30,6 @@ import org.springframework.core.ResolvableType;
import org.springframework.core.io.ClassPathResource;
import org.springframework.core.io.Resource;
import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.core.io.buffer.DataBufferFactory;
import org.springframework.core.io.buffer.DataBufferUtils;
import org.springframework.core.io.buffer.LeakAwareDataBufferFactory;
import org.springframework.core.io.buffer.support.DataBufferTestUtils;
@@ -36,19 +37,18 @@ import org.springframework.core.io.support.ResourceRegion;
import org.springframework.util.MimeType;
import org.springframework.util.MimeTypeUtils;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.nio.charset.StandardCharsets.*;
import static org.junit.Assert.*;
/**
* Test cases for {@link ResourceRegionEncoder} class.
*
* @author Brian Clozel
*/
public class ResourceRegionEncoderTests {
private ResourceRegionEncoder encoder = new ResourceRegionEncoder();
private DataBufferFactory bufferFactory = new LeakAwareDataBufferFactory();
private LeakAwareDataBufferFactory bufferFactory = new LeakAwareDataBufferFactory();
@Test
@@ -79,10 +79,13 @@ public class ResourceRegionEncoderTests {
.consumeNextWith(stringConsumer("Spring"))
.expectComplete()
.verify();
// TODO: https://github.com/reactor/reactor-core/issues/1634
// this.bufferFactory.checkForLeaks();
}
@Test
public void shouldEncodeMultipleResourceRegionsFileResource() throws Exception {
public void shouldEncodeMultipleResourceRegionsFileResource() {
Resource resource = new ClassPathResource("ResourceRegionEncoderTests.txt", getClass());
Flux<ResourceRegion> regions = Flux.just(
new ResourceRegion(resource, 0, 6),
@@ -118,6 +121,33 @@ public class ResourceRegionEncoderTests {
.consumeNextWith(stringConsumer("\r\n--" + boundary + "--"))
.expectComplete()
.verify();
// TODO: https://github.com/reactor/reactor-core/issues/1634
// this.bufferFactory.checkForLeaks();
}
@Test // gh-
public void cancelWithoutDemandForMultipleResourceRegions() {
Resource resource = new ClassPathResource("ResourceRegionEncoderTests.txt", getClass());
Flux<ResourceRegion> regions = Flux.just(
new ResourceRegion(resource, 0, 6),
new ResourceRegion(resource, 7, 9),
new ResourceRegion(resource, 17, 4),
new ResourceRegion(resource, 22, 17)
);
String boundary = MimeTypeUtils.generateMultipartBoundaryString();
Flux<DataBuffer> flux = this.encoder.encode(regions, this.bufferFactory,
ResolvableType.forClass(ResourceRegion.class),
MimeType.valueOf("text/plain"),
Collections.singletonMap(ResourceRegionEncoder.BOUNDARY_STRING_HINT, boundary)
);
ZeroDemandSubscriber subscriber = new ZeroDemandSubscriber();
flux.subscribe(subscriber);
subscriber.cancel();
this.bufferFactory.checkForLeaks();
}
@Test
@@ -142,6 +172,9 @@ public class ResourceRegionEncoderTests {
.consumeNextWith(stringConsumer("Spring"))
.expectError(EncodingException.class)
.verify();
// TODO: https://github.com/reactor/reactor-core/issues/1634
// this.bufferFactory.checkForLeaks();
}
protected Consumer<DataBuffer> stringConsumer(String expected) {
@@ -154,4 +187,12 @@ public class ResourceRegionEncoderTests {
}
private static class ZeroDemandSubscriber extends BaseSubscriber<DataBuffer> {
@Override
protected void hookOnSubscribe(Subscription subscription) {
// Just subscribe without requesting
}
}
}

View File

@@ -123,9 +123,23 @@ public abstract class AbstractDataBufferAllocatingTestCase {
if (this.bufferFactory instanceof NettyDataBufferFactory) {
ByteBufAllocator allocator = ((NettyDataBufferFactory) this.bufferFactory).getByteBufAllocator();
if (allocator instanceof PooledByteBufAllocator) {
PooledByteBufAllocatorMetric metric = ((PooledByteBufAllocator) allocator).metric();
long total = getAllocations(metric.directArenas()) + getAllocations(metric.heapArenas());
assertEquals("ByteBuf Leak: " + total + " unreleased allocations", 0, total);
Instant start = Instant.now();
while (true) {
PooledByteBufAllocatorMetric metric = ((PooledByteBufAllocator) allocator).metric();
long total = getAllocations(metric.directArenas()) + getAllocations(metric.heapArenas());
if (total == 0) {
return;
}
if (Instant.now().isBefore(start.plus(Duration.ofSeconds(5)))) {
try {
Thread.sleep(50);
}
catch (InterruptedException ex) {
// ignore
}
}
assertEquals("ByteBuf Leak: " + total + " unreleased allocations", 0, total);
}
}
}
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -35,8 +35,11 @@ import java.util.concurrent.CountDownLatch;
import io.netty.buffer.ByteBuf;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.mockito.stubbing.Answer;
import org.reactivestreams.Subscription;
import reactor.core.publisher.BaseSubscriber;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
@@ -184,6 +187,20 @@ public class DataBufferUtilsTests extends AbstractDataBufferAllocatingTestCase {
.verify();
}
// TODO: Remove ignore after https://github.com/reactor/reactor-core/issues/1634
@Ignore
@Test // gh-22107
public void readAsynchronousFileChannelCancelWithoutDemand() throws Exception {
URI uri = this.resource.getURI();
Flux<DataBuffer> flux = DataBufferUtils.readAsynchronousFileChannel(
() -> AsynchronousFileChannel.open(Paths.get(uri), StandardOpenOption.READ),
this.bufferFactory, 3);
BaseSubscriber<DataBuffer> subscriber = new ZeroDemandSubscriber();
flux.subscribe(subscriber);
subscriber.cancel();
}
@Test
public void readResource() throws Exception {
Flux<DataBuffer> flux = DataBufferUtils.read(this.resource, this.bufferFactory, 3);
@@ -735,5 +752,12 @@ public class DataBufferUtilsTests extends AbstractDataBufferAllocatingTestCase {
}
private static class ZeroDemandSubscriber extends BaseSubscriber<DataBuffer> {
@Override
protected void hookOnSubscribe(Subscription subscription) {
// Just subscribe without requesting
}
}
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -17,9 +17,14 @@
package org.springframework.core.io.buffer;
import java.nio.ByteBuffer;
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.jetbrains.annotations.NotNull;
import org.junit.After;
@@ -37,6 +42,9 @@ import org.springframework.util.Assert;
*/
public class LeakAwareDataBufferFactory implements DataBufferFactory {
private static final Log logger = LogFactory.getLog(LeakAwareDataBufferFactory.class);
private final DataBufferFactory delegate;
private final List<LeakAwareDataBuffer> created = new ArrayList<>();
@@ -65,13 +73,27 @@ public class LeakAwareDataBufferFactory implements DataBufferFactory {
* method.
*/
public void checkForLeaks() {
this.created.stream()
.filter(LeakAwareDataBuffer::isAllocated)
.findFirst()
.map(LeakAwareDataBuffer::leakError)
.ifPresent(leakError -> {
throw leakError;
});
Instant start = Instant.now();
while (true) {
if (this.created.stream().noneMatch(LeakAwareDataBuffer::isAllocated)) {
return;
}
if (Instant.now().isBefore(start.plus(Duration.ofSeconds(5)))) {
try {
Thread.sleep(50);
}
catch (InterruptedException ex) {
// ignore
}
}
List<AssertionError> errors = this.created.stream()
.filter(LeakAwareDataBuffer::isAllocated)
.map(LeakAwareDataBuffer::leakError)
.collect(Collectors.toList());
errors.forEach(it -> logger.error("Leaked error: ", it));
throw new AssertionError(errors.size() + " buffer leaks detected (see logs above)");
}
}
@Override