Fix a bug where AbstractConstant.compareTo() returns 0 for different constants

Related issue: #2354

Motivation:

AbstractConstant.compareTo() can return 0 even if the specified constant
object is not the same instance with 'this'.

Modifications:

- Compare the identityHashCode of constant first. If that fails,
  allocate a small direct buffer and use its memory address as a unique
  value.  If the platform does not provide a way to get the memory
  address of a direct buffer, use a thread-local random value.
- Signal cannot extend AbstractConstant. Use delegation.

Result:

It is practically impossible for AbstractConstant.compareTo() to return
0 for different constant objects.
This commit is contained in:
Trustin Lee 2014-07-29 14:56:28 -07:00
parent 410b5623ff
commit e64644b21c
2 changed files with 59 additions and 21 deletions

View File

@ -15,6 +15,11 @@
*/
package io.netty.util;
import io.netty.util.internal.PlatformDependent;
import io.netty.util.internal.ThreadLocalRandom;
import java.nio.ByteBuffer;
/**
* Base implementation of {@link Constant}.
*/
@ -22,6 +27,8 @@ public abstract class AbstractConstant<T extends AbstractConstant<T>> implements
private final int id;
private final String name;
private volatile long uniquifier;
private ByteBuffer directBuffer;
/**
* Creates a new instance.
@ -41,32 +48,64 @@ public abstract class AbstractConstant<T extends AbstractConstant<T>> implements
return id;
}
@Override
public final String toString() {
return name();
}
@Override
public final int hashCode() {
return super.hashCode();
}
@Override
public final boolean equals(Object o) {
return super.equals(o);
public final boolean equals(Object obj) {
return super.equals(obj);
}
@Override
public final int compareTo(T other) {
if (this == other) {
public final int compareTo(T o) {
if (this == o) {
return 0;
}
int returnCode = name.compareTo(other.name());
@SuppressWarnings("UnnecessaryLocalVariable")
AbstractConstant<T> other = o;
int returnCode;
returnCode = hashCode() - other.hashCode();
if (returnCode != 0) {
return returnCode;
}
return ((Integer) id).compareTo(other.id());
long thisUV = uniquifier();
long otherUV = other.uniquifier();
if (thisUV < otherUV) {
return -1;
}
if (thisUV > otherUV) {
return 1;
}
throw new Error("failed to compare two different constants");
}
@Override
public final String toString() {
return name();
private long uniquifier() {
long uniquifier;
if ((uniquifier = this.uniquifier) == 0) {
synchronized (this) {
while ((uniquifier = this.uniquifier) == 0) {
if (PlatformDependent.hasUnsafe()) {
directBuffer = ByteBuffer.allocateDirect(1);
this.uniquifier = PlatformDependent.directBufferAddress(directBuffer);
} else {
directBuffer = null;
this.uniquifier = ThreadLocalRandom.current().nextLong();
}
}
}
}
return uniquifier;
}
}

View File

@ -45,15 +45,13 @@ public final class Signal extends Error implements Constant<Signal> {
return pool.valueOf(firstNameComponent, secondNameComponent);
}
private final int id;
private final String name;
private final SignalConstant constant;
/**
* Creates a new {@link Signal} with the specified {@code name}.
*/
private Signal(int id, String name) {
this.id = id;
this.name = name;
constant = new SignalConstant(id, name);
}
/**
@ -78,12 +76,12 @@ public final class Signal extends Error implements Constant<Signal> {
@Override
public int id() {
return id;
return constant.id();
}
@Override
public String name() {
return name;
return constant.name();
}
@Override
@ -102,16 +100,17 @@ public final class Signal extends Error implements Constant<Signal> {
return 0;
}
int returnCode = name.compareTo(other.name());
if (returnCode != 0) {
return returnCode;
}
return ((Integer) id).compareTo(other.id());
return constant.compareTo(other.constant);
}
@Override
public String toString() {
return name();
}
private static final class SignalConstant extends AbstractConstant<SignalConstant> {
SignalConstant(int id, String name) {
super(id, name);
}
}
}