Skip to content

Commit

Permalink
Create a new ByteStore when writing zero sized arrays. Fix #384
Browse files Browse the repository at this point in the history
  • Loading branch information
minborg committed Jun 2, 2022
1 parent 8623b17 commit 23af23b
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 4 deletions.
10 changes: 6 additions & 4 deletions src/main/java/net/openhft/chronicle/bytes/NativeBytes.java
Expand Up @@ -17,7 +17,9 @@
*/
package net.openhft.chronicle.bytes;

import net.openhft.chronicle.bytes.internal.EmptyByteStore;
import net.openhft.chronicle.bytes.internal.ReferenceCountedUtil;
import net.openhft.chronicle.bytes.internal.SingletonEmptyByteStore;
import net.openhft.chronicle.bytes.util.DecoratedBufferOverflowException;
import net.openhft.chronicle.core.Jvm;
import net.openhft.chronicle.core.Maths;
Expand Down Expand Up @@ -57,8 +59,7 @@ public NativeBytes(@NotNull final BytesStore store, @NonNegative final long capa

public NativeBytes(@NotNull final BytesStore store)
throws IllegalStateException, IllegalArgumentException {
super(store, 0, store.capacity());
capacity = store.capacity();
this(store, store.capacity());
}

/**
Expand Down Expand Up @@ -160,7 +161,8 @@ protected void writeCheckOffset(final @NonNegative long offset, final @NonNegati
throws BufferOverflowException, IllegalStateException {
if (offset >= bytesStore.start() && offset + adding >= bytesStore.start()) {
final long writeEnd = offset + adding;
if (writeEnd <= bytesStore.safeLimit()) {
// Always resize if we are backed by a SingletonEmptyByteStore as this is shared and does not provide all functionality
if (writeEnd <= bytesStore.safeLimit() && !(EmptyByteStore.class.isInstance(bytesStore))) {
return; // do nothing.
}
if (writeEnd >= capacity)
Expand Down Expand Up @@ -226,7 +228,7 @@ private void resize(@NonNegative final long endOfBuffer)
if (endOfBuffer > capacity())
throw new DecoratedBufferOverflowException(endOfBuffer + ">" + capacity());
final long realCapacity = realCapacity();
if (endOfBuffer <= realCapacity) {
if (endOfBuffer <= realCapacity && !(EmptyByteStore.class.isInstance(bytesStore))) {
// No resize
return;
}
Expand Down
@@ -0,0 +1,68 @@
/*
* Copyright (c) 2016-2022 chronicle.software
*
* https://chronicle.software
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package net.openhft.chronicle.bytes.issue;

import net.openhft.chronicle.bytes.Bytes;
import net.openhft.chronicle.bytes.VanillaBytes;
import net.openhft.chronicle.bytes.internal.EmptyByteStore;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import java.util.stream.Stream;

import static org.junit.jupiter.api.Assertions.assertTrue;

final class Issue384ReplaceByteStoreOnEmptyArrayTest {

@ParameterizedTest(name = "{index}: ({0})")
@MethodSource("bytesToTest")
void reproduce(String classSimpleName, Bytes<?> bytes) {

// throwing AssertionErrors in the code makes testing more complicated.
boolean replacedOrRefused = false;
try {
// Write an empty array into the empty bytes. This should trigger a
// resize since the empty bytes is using a shared backing EmptyByteStore.
bytes.write(new byte[0]);
// Make sure we are not using an EmptyByteStore
replacedOrRefused = !(bytes.bytesStore() instanceof EmptyByteStore);
} catch (AssertionError ignored) {
// This is ok as some of the Bytes objects are not elastic.
replacedOrRefused = true;
} finally {
bytes.releaseLast();
}
assertTrue(replacedOrRefused);
}

static Stream<Arguments> bytesToTest() {
return Stream.of(
Bytes.allocateElasticDirect(),
Bytes.allocateElasticDirect(0),
Bytes.allocateDirect(0),
Bytes.allocateElasticOnHeap(0),
VanillaBytes.vanillaBytes(),
Bytes.from(""),
Bytes.allocateDirect(new byte[0]),
Bytes.fromDirect("")
)
.map(b -> Arguments.of(b.getClass().getSimpleName() + (b.isElastic() ? " Elastic" : " Fixed"), b));
}

}

0 comments on commit 23af23b

Please sign in to comment.