Skip to content

Commit

Permalink
Replace UniqueName with Constant and ConstantPool
Browse files Browse the repository at this point in the history
- Proposed fix for netty#1824

UniqueName and its subtypes do not allow getting the previously registered instance.  For example, let's assume that a user is running his/her application in an OSGi container with Netty bundles and his server bundle.  Whenever the server bundle is reloaded, the server will try to create a new AttributeKey instance with the same name.  However, Netty bundles were not reloaded at all, so AttributeKey will complain that the name is taken already (by the previously loaded bundle.)

To fix this problem:

- Replaced UniqueName with Constant, AbstractConstant, and ConstantPool.  Better name and better design.

- Sctp/Udt/RxtxChannelOption is not a ChannelOption anymore.  They are just constant providers and ChannelOption is final now.  It's because caching anything that's from outside of netty-transport will lead to ClassCastException on reload, because ChannelOption's constant pool will keep all option objects for reuse.

- Signal implements Constant because we can't ensure its uniqueness anymore by relying on the exception raised by UniqueName's constructor.
  • Loading branch information
trustin committed Oct 25, 2013
1 parent 98541be commit dc009b2
Show file tree
Hide file tree
Showing 23 changed files with 413 additions and 394 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public enum ServerHandshakeStateEvent {
}

private static final AttributeKey<WebSocketServerHandshaker> HANDSHAKER_ATTR_KEY =
new AttributeKey<WebSocketServerHandshaker>(WebSocketServerHandshaker.class.getName());
AttributeKey.valueOf(WebSocketServerHandshaker.class.getName());

private final String websocketPath;
private final String subprotocols;
Expand Down
4 changes: 2 additions & 2 deletions codec/src/main/java/io/netty/handler/codec/DecoderResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@

public class DecoderResult {

protected static final Signal SIGNAL_UNFINISHED = new Signal(DecoderResult.class.getName() + ".UNFINISHED");
protected static final Signal SIGNAL_SUCCESS = new Signal(DecoderResult.class.getName() + ".SUCCESS");
protected static final Signal SIGNAL_UNFINISHED = Signal.valueOf(DecoderResult.class.getName() + ".UNFINISHED");
protected static final Signal SIGNAL_SUCCESS = Signal.valueOf(DecoderResult.class.getName() + ".SUCCESS");

public static final DecoderResult UNFINISHED = new DecoderResult(SIGNAL_UNFINISHED);
public static final DecoderResult SUCCESS = new DecoderResult(SIGNAL_SUCCESS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@
*/
public abstract class ReplayingDecoder<S> extends ByteToMessageDecoder {

static final Signal REPLAY = new Signal(ReplayingDecoder.class.getName() + ".REPLAY");
static final Signal REPLAY = Signal.valueOf(ReplayingDecoder.class.getName() + ".REPLAY");

private final ReplayingDecoderBuffer replayable = new ReplayingDecoderBuffer();
private S state;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import io.netty.channel.ChannelHandlerContext;
import io.netty.util.Attribute;
import io.netty.util.AttributeKey;

import org.jboss.marshalling.MarshallerFactory;
import org.jboss.marshalling.MarshallingConfiguration;
import org.jboss.marshalling.Unmarshaller;
Expand All @@ -35,7 +34,7 @@
*/
public class ContextBoundUnmarshallerProvider extends DefaultUnmarshallerProvider {

private static final AttributeKey<Unmarshaller> UNMARSHALLER = new AttributeKey<Unmarshaller>(
private static final AttributeKey<Unmarshaller> UNMARSHALLER = AttributeKey.valueOf(
ContextBoundUnmarshallerProvider.class.getName() + ".unmarshaller");

public ContextBoundUnmarshallerProvider(MarshallerFactory factory, MarshallingConfiguration config) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
public class CompatibleObjectEncoder extends MessageToByteEncoder<Serializable> {

private static final AttributeKey<ObjectOutputStream> OOS =
new AttributeKey<ObjectOutputStream>(CompatibleObjectEncoder.class.getName() + ".oos");
AttributeKey.valueOf(CompatibleObjectEncoder.class.getName() + ".oos");

private final int resetInterval;
private int writtenObjects;
Expand Down
72 changes: 72 additions & 0 deletions common/src/main/java/io/netty/util/AbstractConstant.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* 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.util;

/**
* Base implementation of {@link Constant}.
*/
public abstract class AbstractConstant<T extends AbstractConstant<T>> implements Constant<T> {

private final int id;
private final String name;

/**
* Creates a new instance.
*/
protected AbstractConstant(int id, String name) {
this.id = id;
this.name = name;
}

@Override
public final String name() {
return name;
}

@Override
public final int id() {
return id;
}

@Override
public final int hashCode() {
return super.hashCode();
}

@Override
public final boolean equals(Object o) {
return super.equals(o);
}

@Override
public final int compareTo(T other) {
if (this == other) {
return 0;
}

int returnCode = name.compareTo(other.name());
if (returnCode != 0) {
return returnCode;
}

return ((Integer) id).compareTo(other.id());
}

@Override
public final String toString() {
return name();
}
}
27 changes: 15 additions & 12 deletions common/src/main/java/io/netty/util/AttributeKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,28 +15,31 @@
*/
package io.netty.util;

import io.netty.util.internal.PlatformDependent;

import java.util.concurrent.ConcurrentMap;

/**
* Key which can be used to access {@link Attribute} out of the {@link AttributeMap}. Be aware that it is not be
* possible to have multiple keys with the same name.
*
*
* @param <T> the type of the {@link Attribute} which can be accessed via this {@link AttributeKey}.
*/
@SuppressWarnings("UnusedDeclaration") // 'T' is used only at compile time
public final class AttributeKey<T> extends UniqueName {
public final class AttributeKey<T> extends AbstractConstant<AttributeKey<T>> {

private static final ConcurrentMap<String, Boolean> names = PlatformDependent.newConcurrentHashMap();
private static final ConstantPool<AttributeKey<Object>> pool = new ConstantPool<AttributeKey<Object>>() {
@Override
protected AttributeKey<Object> newConstant(int id, String name) {
return new AttributeKey<Object>(id, name);
}
};

/**
* Create a new instance
*
* @param name the name under which the {@link AttributeKey} will be registered
* Returns the singleton instance of the {@link AttributeKey} which has the specified {@code name}.
*/
public AttributeKey(String name) {
super(names, name);
@SuppressWarnings("unchecked")
public static <T> AttributeKey<T> valueOf(String name) {
return (AttributeKey<T>) pool.valueOf(name);
}

private AttributeKey(int id, String name) {
super(id, name);
}
}
32 changes: 32 additions & 0 deletions common/src/main/java/io/netty/util/Constant.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* 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.util;

/**
* A singleton which is safe to compare via the {@code ==} operator. Created and managed by {@link ConstantPool}.
*/
public interface Constant<T extends Constant<T>> extends Comparable<T> {

/**
* Returns the unique number assigned to this {@link Constant}.
*/
int id();

/**
* Returns the name of this {@link Constant}.
*/
String name();
}
54 changes: 54 additions & 0 deletions common/src/main/java/io/netty/util/ConstantPool.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Copyright 2013 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.util;

import java.util.HashMap;
import java.util.Map;

/**
* A pool of {@link Constant}s.
*
* @param <T> the type of the constant
*/
public abstract class ConstantPool<T extends Constant<T>> {

private final Map<String, T> constants = new HashMap<String, T>();

private int nextId = 1;

/**
* Creates a new or the existing {@link Constant} mapped to the specified {@code name}.
*/
public T valueOf(String name) {
if (name == null) {
throw new NullPointerException("name");
}

synchronized (constants) {
T c = constants.get(name);
if (c == null) {
c = newConstant(nextId, name);
constants.put(name, c);
nextId ++;
}

return c;
}
}

protected abstract T newConstant(int id, String name);
}
65 changes: 54 additions & 11 deletions common/src/main/java/io/netty/util/Signal.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,30 +16,39 @@
package io.netty.util;


import io.netty.util.internal.PlatformDependent;

import java.util.concurrent.ConcurrentMap;

/**
* A special {@link Error} which is used to signal some state or request by throwing it.
* {@link Signal} has an empty stack trace and has no cause to save the instantiation overhead.
*/
public final class Signal extends Error {
public final class Signal extends Error implements Constant<Signal> {

private static final long serialVersionUID = -221145131122459977L;

private static final ConcurrentMap<String, Boolean> map = PlatformDependent.newConcurrentHashMap();
private static final ConstantPool<Signal> pool = new ConstantPool<Signal>() {
@Override
protected Signal newConstant(int id, String name) {
return new Signal(id, name);
}
};

/**
* Returns the {@link Signal} of the specified name.
*/
public static Signal valueOf(String name) {
return pool.valueOf(name);
}

private final UniqueName uname;
private final int id;
private final String name;

/**
* Create a new instance
*
* @param name the name under which it is registered
*/
public Signal(String name) {
super(name);
uname = new UniqueName(map, name);
private Signal(int id, String name) {
this.id = id;
this.name = name;
}

/**
Expand All @@ -62,8 +71,42 @@ public Throwable fillInStackTrace() {
return this;
}

@Override
public int id() {
return id;
}

@Override
public String name() {
return name;
}

@Override
public boolean equals(Object obj) {
return this == obj;
}

@Override
public int hashCode() {
return System.identityHashCode(this);
}

@Override
public int compareTo(Signal other) {
if (this == other) {
return 0;
}

int returnCode = name.compareTo(other.name());
if (returnCode != 0) {
return returnCode;
}

return ((Integer) id).compareTo(other.id());
}

@Override
public String toString() {
return uname.name();
return name();
}
}
Loading

0 comments on commit dc009b2

Please sign in to comment.