Permalink
Browse files

Remove ChannelBuf/ByteBuf.Unsafe

- Fixes #826
Unsafe.isFreed(), free(), suspend/resumeIntermediaryAllocations() are not that dangerous. internalNioBuffer() and internalNioBuffers() are dangerous but it seems like nobody is using it even inside Netty. Removing those two methods also removes the necessity to keep Unsafe interface at all.
  • Loading branch information...
trustin committed Dec 17, 2012
1 parent 33134b1 commit 03e68482bba54aa0c0d35649ac9c090b0ab75473
Showing with 225 additions and 332 deletions.
  1. +21 −0 buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java
  2. +25 −46 buffer/src/main/java/io/netty/buffer/ByteBuf.java
  3. +7 −15 buffer/src/main/java/io/netty/buffer/ChannelBuf.java
  4. +8 −0 buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
  5. +10 −32 buffer/src/main/java/io/netty/buffer/DefaultCompositeByteBuf.java
  6. +1 −8 buffer/src/main/java/io/netty/buffer/DefaultMessageBuf.java
  7. +9 −25 buffer/src/main/java/io/netty/buffer/DuplicatedByteBuf.java
  8. +40 −0 buffer/src/main/java/io/netty/buffer/IllegalMemoryAccessException.java
  9. +8 −32 buffer/src/main/java/io/netty/buffer/PooledByteBuf.java
  10. +1 −8 buffer/src/main/java/io/netty/buffer/QueueBackedMessageBuf.java
  11. +9 −25 buffer/src/main/java/io/netty/buffer/ReadOnlyByteBuf.java
  12. +9 −25 buffer/src/main/java/io/netty/buffer/SlicedByteBuf.java
  13. +28 −5 buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java
  14. +7 −30 buffer/src/main/java/io/netty/buffer/UnpooledDirectByteBuf.java
  15. +8 −30 buffer/src/main/java/io/netty/buffer/UnpooledHeapByteBuf.java
  16. +1 −1 codec/src/main/java/io/netty/handler/codec/ByteToByteDecoder.java
  17. +1 −1 codec/src/main/java/io/netty/handler/codec/ByteToByteEncoder.java
  18. +2 −2 codec/src/main/java/io/netty/handler/codec/ByteToMessageDecoder.java
  19. +2 −2 codec/src/main/java/io/netty/handler/codec/MessageToMessageCodec.java
  20. +1 −1 codec/src/main/java/io/netty/handler/codec/ReplayingDecoder.java
  21. +5 −21 codec/src/main/java/io/netty/handler/codec/ReplayingDecoderBuffer.java
  22. +1 −1 codec/src/main/java/io/netty/handler/codec/serialization/CompatibleObjectEncoder.java
  23. +2 −2 handler/src/main/java/io/netty/handler/logging/ByteLoggingHandler.java
  24. +4 −4 handler/src/main/java/io/netty/handler/ssl/SslHandler.java
  25. +2 −2 microbench/src/test/java/io/netty/microbench/buffer/ByteBufAllocatorBenchmark.java
  26. +1 −1 transport/src/main/java/io/netty/channel/ChannelInboundByteHandlerAdapter.java
  27. +1 −1 transport/src/main/java/io/netty/channel/ChannelInboundHandlerAdapter.java
  28. +1 −1 transport/src/main/java/io/netty/channel/ChannelOutboundHandlerAdapter.java
  29. +2 −3 transport/src/main/java/io/netty/channel/DefaultChannelHandlerContext.java
  30. +1 −1 transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java
  31. +4 −4 transport/src/main/java/io/netty/channel/socket/aio/AioSocketChannel.java
  32. +3 −3 transport/src/test/java/io/netty/channel/local/LocalTransportThreadModelTest.java
@@ -171,6 +171,27 @@ public ByteBuf discardReadBytes() {
return this;
}
+ @Override
+ public ByteBuf discardSomeReadBytes() {
+ if (readerIndex == 0) {
+ return this;
+ }
+
+ if (readerIndex == writerIndex) {
+ adjustMarkers(readerIndex);
+ writerIndex = readerIndex = 0;
+ return this;
+ }
+
+ if (readerIndex >= capacity() >>> 1) {
+ setBytes(0, this, readerIndex, writerIndex - readerIndex);
+ writerIndex -= readerIndex;
+ adjustMarkers(readerIndex);
+ readerIndex = 0;
+ }
+ return this;
+ }
+
protected void adjustMarkers(int decrement) {
markedReaderIndex = Math.max(markedReaderIndex - decrement, 0);
markedWriterIndex = Math.max(markedWriterIndex - decrement, 0);
@@ -457,6 +457,14 @@
*/
ByteBuf discardReadBytes();
+ /**
+ * Similar to {@link ByteBuf#discardReadBytes()} except that this method might discard
+ * some, all, or none of read bytes depending on its internal implementation to reduce
+ * overall memory bandwidth consumption at the cost of potentially additional memory
+ * consumption.
+ */
+ ByteBuf discardSomeReadBytes();
+
/**
* Makes sure the number of {@linkplain #writableBytes() the writable bytes}
* is equal to or greater than the specified value. If there is enough
@@ -1827,6 +1835,23 @@
*/
String toString(int index, int length, Charset charset);
+ /**
+ * Suspends the intermediary deallocation of the internal memory block of this buffer until asked via
+ * {@link #resumeIntermediaryDeallocations()}. An intermediary deallocation is usually made when the capacity of
+ * a buffer changes.
+ *
+ * @throws UnsupportedOperationException if this buffer is derived
+ */
+ ByteBuf suspendIntermediaryDeallocations();
+
+ /**
+ * Resumes the intermediary deallocation of the internal memory block of this buffer, suspended by
+ * {@link #suspendIntermediaryDeallocations()}.
+ *
+ * @throws UnsupportedOperationException if this buffer is derived
+ */
+ ByteBuf resumeIntermediaryDeallocations();
+
/**
* Returns a hash code which was calculated from the content of this
* buffer. If there's a byte array which is
@@ -1868,50 +1893,4 @@
*/
@Override
String toString();
-
- /**
- * Provides access to potentially unsafe operations of this buffer.
- */
- @Override
- Unsafe unsafe();
-
- /**
- * Provides the potentially unsafe operations of {@link ByteBuf}.
- */
- interface Unsafe extends ChannelBuf.Unsafe {
- /**
- * Returns the internal NIO buffer that is reused for I/O.
- *
- * @throws UnsupportedOperationException if the buffer has no internal NIO buffer
- */
- ByteBuffer internalNioBuffer();
-
- /**
- * Returns the internal NIO buffer array that is reused for I/O.
- *
- * @throws UnsupportedOperationException if the buffer has no internal NIO buffer array
- */
- ByteBuffer[] internalNioBuffers();
-
- /**
- * Similar to {@link ByteBuf#discardReadBytes()} except that this method might discard
- * some, all, or none of read bytes depending on its internal implementation to reduce
- * overall memory bandwidth consumption at the cost of potentially additional memory
- * consumption.
- */
- void discardSomeReadBytes();
-
- /**
- * Suspends the intermediary deallocation of the internal memory block of this buffer until asked via
- * {@link #resumeIntermediaryDeallocations()}. An intermediary deallocation is usually made when the capacity of
- * a buffer changes.
- */
- void suspendIntermediaryDeallocations();
-
- /**
- * Resumes the intermediary deallocation of the internal memory block of this buffer, suspended by
- * {@link #suspendIntermediaryDeallocations()}.
- */
- void resumeIntermediaryDeallocations();
- }
}
@@ -22,23 +22,15 @@
ChannelBufType type();
/**
- * Provides access to potentially unsafe operations of this buffer.
+ * Returns {@code true} if and only if this buffer has been deallocated by {@link #free()}.
*/
- Unsafe unsafe();
+ boolean isFreed();
/**
- * Provides the potentially unsafe operations of {@link ByteBuf}.
+ * Deallocates the internal memory block of this buffer or returns it to the allocator or pool it came from.
+ * The result of accessing a released buffer is unspecified and can even cause JVM crash.
+ *
+ * @throws UnsupportedOperationException if this buffer is derived
*/
- interface Unsafe {
- /**
- * Returns {@code true} if and only if this buffer has been deallocated by {@link #free()}.
- */
- boolean isFreed();
-
- /**
- * Deallocates the internal memory block of this buffer or returns it to the {@link ByteBufAllocator} it came
- * from. The result of accessing a released buffer is unspecified and can even cause JVM crash.
- */
- void free();
- }
+ void free();
}
@@ -81,6 +81,9 @@
@Override
CompositeByteBuf discardReadBytes();
+ @Override
+ CompositeByteBuf discardSomeReadBytes();
+
@Override
CompositeByteBuf ensureWritableBytes(int minWritableBytes);
@@ -225,4 +228,9 @@
@Override
CompositeByteBuf writeZero(int length);
+ @Override
+ CompositeByteBuf suspendIntermediaryDeallocations();
+
+ @Override
+ CompositeByteBuf resumeIntermediaryDeallocations();
}
@@ -15,7 +15,6 @@
*/
package io.netty.buffer;
-import io.netty.buffer.ByteBuf.Unsafe;
import io.netty.util.internal.DetectionUtil;
import java.io.IOException;
@@ -40,7 +39,7 @@
* is recommended to use {@link Unpooled#wrappedBuffer(ByteBuf...)}
* instead of calling the constructor explicitly.
*/
-public class DefaultCompositeByteBuf extends AbstractByteBuf implements CompositeByteBuf, Unsafe {
+public class DefaultCompositeByteBuf extends AbstractByteBuf implements CompositeByteBuf {
private static final ByteBuffer[] EMPTY_NIOBUFFERS = new ByteBuffer[0];
@@ -1292,7 +1291,7 @@ void freeIfNecessary() {
}
if (suspendedDeallocations == null) {
- buf.unsafe().free(); // We should not get a NPE here. If so, it must be a bug.
+ buf.free(); // We should not get a NPE here. If so, it must be a bug.
} else {
suspendedDeallocations.add(buf);
}
@@ -1525,26 +1524,8 @@ public CompositeByteBuf writeZero(int length) {
}
@Override
- public ByteBuffer internalNioBuffer() {
- if (components.size() == 1) {
- return components.get(0).buf.unsafe().internalNioBuffer();
- }
- throw new UnsupportedOperationException();
- }
-
- @Override
- public ByteBuffer[] internalNioBuffers() {
- ByteBuffer[] nioBuffers = new ByteBuffer[components.size()];
- int index = 0;
- for (Component component : components) {
- nioBuffers[index++] = component.buf.unsafe().internalNioBuffer();
- }
- return nioBuffers;
- }
-
- @Override
- public void discardSomeReadBytes() {
- discardReadComponents();
+ public CompositeByteBuf discardSomeReadBytes() {
+ return discardReadComponents();
}
@Override
@@ -1566,33 +1547,30 @@ public void free() {
}
@Override
- public void suspendIntermediaryDeallocations() {
+ public CompositeByteBuf suspendIntermediaryDeallocations() {
if (suspendedDeallocations == null) {
suspendedDeallocations = new ArrayDeque<ByteBuf>(2);
}
+ return this;
}
@Override
- public void resumeIntermediaryDeallocations() {
+ public CompositeByteBuf resumeIntermediaryDeallocations() {
if (suspendedDeallocations == null) {
- return;
+ return this;
}
Queue<ByteBuf> suspendedDeallocations = this.suspendedDeallocations;
this.suspendedDeallocations = null;
for (ByteBuf buf: suspendedDeallocations) {
- buf.unsafe().free();
+ buf.free();
}
+ return this;
}
@Override
public ByteBuf unwrap() {
return null;
}
-
- @Override
- public Unsafe unsafe() {
- return this;
- }
}
@@ -15,12 +15,10 @@
*/
package io.netty.buffer;
-import io.netty.buffer.ChannelBuf.Unsafe;
-
import java.util.ArrayDeque;
import java.util.Collection;
-final class DefaultMessageBuf<T> extends ArrayDeque<T> implements MessageBuf<T>, Unsafe {
+final class DefaultMessageBuf<T> extends ArrayDeque<T> implements MessageBuf<T> {
private static final long serialVersionUID = 1229808623624907552L;
@@ -65,11 +63,6 @@ public int drainTo(Collection<? super T> c, int maxElements) {
return cnt;
}
- @Override
- public Unsafe unsafe() {
- return this;
- }
-
@Override
public boolean isFreed() {
return freed;
@@ -15,8 +15,6 @@
*/
package io.netty.buffer;
-import io.netty.buffer.ByteBuf.Unsafe;
-
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
@@ -31,7 +29,7 @@
* parent. It is recommended to use {@link ByteBuf#duplicate()} instead
* of calling the constructor explicitly.
*/
-public class DuplicatedByteBuf extends AbstractByteBuf implements Unsafe {
+public class DuplicatedByteBuf extends AbstractByteBuf {
private final ByteBuf buffer;
@@ -235,37 +233,23 @@ public ByteBuffer nioBuffer(int index, int length) {
}
@Override
- public ByteBuffer internalNioBuffer() {
- return buffer.unsafe().internalNioBuffer();
- }
-
- @Override
- public ByteBuffer[] internalNioBuffers() {
- return buffer.unsafe().internalNioBuffers();
+ public boolean isFreed() {
+ return buffer.isFreed();
}
@Override
- public void discardSomeReadBytes() {
- throw new UnsupportedOperationException();
+ public void free() {
+ throw new UnsupportedOperationException("derived");
}
@Override
- public boolean isFreed() {
- return buffer.unsafe().isFreed();
+ public ByteBuf suspendIntermediaryDeallocations() {
+ throw new UnsupportedOperationException("derived");
}
@Override
- public void free() { }
-
- @Override
- public void suspendIntermediaryDeallocations() { }
-
- @Override
- public void resumeIntermediaryDeallocations() { }
-
- @Override
- public Unsafe unsafe() {
- return this;
+ public ByteBuf resumeIntermediaryDeallocations() {
+ throw new UnsupportedOperationException("derived");
}
}
@@ -0,0 +1,40 @@
+/*
+ * Copyright 2012 The Netty Project
+ *
+ * The Netty Project licenses this file to you 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 io.netty.buffer;
+
+/**
+ * A {@link RuntimeException} raised by a {@link ByteBuf} where a user requested an operation on a freed
+ * {@link ByteBuf}.
+ */
+public class IllegalMemoryAccessException extends RuntimeException {
+
+ private static final long serialVersionUID = -6734326916218551327L;
+
+ public IllegalMemoryAccessException() { }
+
+ public IllegalMemoryAccessException(String message) {
+ super(message);
+ }
+
+ public IllegalMemoryAccessException(String message, Throwable cause) {
+ super(message, cause);
+ }
+
+ public IllegalMemoryAccessException(Throwable cause) {
+ super(cause);
+ }
+}
Oops, something went wrong.

0 comments on commit 03e6848

Please sign in to comment.