From e64644b21cf628d6bb1cc967b8066d8274e3e17e Mon Sep 17 00:00:00 2001
From: Trustin Lee <t@motd.kr>
Date: Tue, 29 Jul 2014 14:56:28 -0700
Subject: [PATCH] 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.
---
 .../java/io/netty/util/AbstractConstant.java  | 57 ++++++++++++++++---
 .../src/main/java/io/netty/util/Signal.java   | 23 ++++----
 2 files changed, 59 insertions(+), 21 deletions(-)

diff --git a/common/src/main/java/io/netty/util/AbstractConstant.java b/common/src/main/java/io/netty/util/AbstractConstant.java
index 580c10ed19..040699bbf9 100644
--- a/common/src/main/java/io/netty/util/AbstractConstant.java
+++ b/common/src/main/java/io/netty/util/AbstractConstant.java
@@ -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;
     }
 }
diff --git a/common/src/main/java/io/netty/util/Signal.java b/common/src/main/java/io/netty/util/Signal.java
index 320c1d6e7b..7edf8eecd1 100644
--- a/common/src/main/java/io/netty/util/Signal.java
+++ b/common/src/main/java/io/netty/util/Signal.java
@@ -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);
+        }
+    }
 }