From ad3d04a30c217367c574e22f0d09789df587285e Mon Sep 17 00:00:00 2001 From: Doug Lea Date: Wed, 23 Feb 2011 14:56:44 +0000 Subject: [PATCH] 7017493: ConcurrentLinkedDeque: Unexpected initialization order can lead to crash due to use of Unsafe Reviewed-by: chegar --- .../concurrent/ConcurrentLinkedDeque.java | 71 +++++++++------- .../concurrent/ConcurrentLinkedQueue.java | 52 +++++++----- .../concurrent/ConcurrentSkipListMap.java | 61 +++++++++----- .../concurrent/ConcurrentSkipListSet.java | 20 +++-- .../util/concurrent/CopyOnWriteArrayList.java | 19 +++-- .../util/concurrent/LinkedTransferQueue.java | 60 +++++++------ .../classes/java/util/concurrent/Phaser.java | 20 ++--- .../concurrent/PriorityBlockingQueue.java | 23 ++--- .../util/concurrent/SynchronousQueue.java | 84 ++++++++++++++----- 9 files changed, 245 insertions(+), 165 deletions(-) diff --git a/jdk/src/share/classes/java/util/concurrent/ConcurrentLinkedDeque.java b/jdk/src/share/classes/java/util/concurrent/ConcurrentLinkedDeque.java index 2158084f9f9..1030fac0423 100644 --- a/jdk/src/share/classes/java/util/concurrent/ConcurrentLinkedDeque.java +++ b/jdk/src/share/classes/java/util/concurrent/ConcurrentLinkedDeque.java @@ -272,13 +272,6 @@ public class ConcurrentLinkedDeque private static final Node PREV_TERMINATOR, NEXT_TERMINATOR; - static { - PREV_TERMINATOR = new Node(null); - PREV_TERMINATOR.next = PREV_TERMINATOR; - NEXT_TERMINATOR = new Node(null); - NEXT_TERMINATOR.prev = NEXT_TERMINATOR; - } - @SuppressWarnings("unchecked") Node prevTerminator() { return (Node) PREV_TERMINATOR; @@ -294,6 +287,9 @@ public class ConcurrentLinkedDeque volatile E item; volatile Node next; + Node() { // default constructor for NEXT_TERMINATOR, PREV_TERMINATOR + } + /** * Constructs a new node. Uses relaxed write because item can * only be seen after publication via casNext or casPrev. @@ -324,14 +320,25 @@ public class ConcurrentLinkedDeque // Unsafe mechanics - private static final sun.misc.Unsafe UNSAFE = - sun.misc.Unsafe.getUnsafe(); - private static final long prevOffset = - objectFieldOffset(UNSAFE, "prev", Node.class); - private static final long itemOffset = - objectFieldOffset(UNSAFE, "item", Node.class); - private static final long nextOffset = - objectFieldOffset(UNSAFE, "next", Node.class); + private static final sun.misc.Unsafe UNSAFE; + private static final long prevOffset; + private static final long itemOffset; + private static final long nextOffset; + + static { + try { + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = Node.class; + prevOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("prev")); + itemOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("item")); + nextOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("next")); + } catch (Exception e) { + throw new Error(e); + } + } } /** @@ -1422,14 +1429,6 @@ public class ConcurrentLinkedDeque initHeadTail(h, t); } - // Unsafe mechanics - - private static final sun.misc.Unsafe UNSAFE = - sun.misc.Unsafe.getUnsafe(); - private static final long headOffset = - objectFieldOffset(UNSAFE, "head", ConcurrentLinkedDeque.class); - private static final long tailOffset = - objectFieldOffset(UNSAFE, "tail", ConcurrentLinkedDeque.class); private boolean casHead(Node cmp, Node val) { return UNSAFE.compareAndSwapObject(this, headOffset, cmp, val); @@ -1439,15 +1438,25 @@ public class ConcurrentLinkedDeque return UNSAFE.compareAndSwapObject(this, tailOffset, cmp, val); } - static long objectFieldOffset(sun.misc.Unsafe UNSAFE, - String field, Class klazz) { + // Unsafe mechanics + + private static final sun.misc.Unsafe UNSAFE; + private static final long headOffset; + private static final long tailOffset; + static { + PREV_TERMINATOR = new Node(); + PREV_TERMINATOR.next = PREV_TERMINATOR; + NEXT_TERMINATOR = new Node(); + NEXT_TERMINATOR.prev = NEXT_TERMINATOR; try { - return UNSAFE.objectFieldOffset(klazz.getDeclaredField(field)); - } catch (NoSuchFieldException e) { - // Convert Exception to corresponding Error - NoSuchFieldError error = new NoSuchFieldError(field); - error.initCause(e); - throw error; + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = ConcurrentLinkedDeque.class; + headOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("head")); + tailOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("tail")); + } catch (Exception e) { + throw new Error(e); } } } diff --git a/jdk/src/share/classes/java/util/concurrent/ConcurrentLinkedQueue.java b/jdk/src/share/classes/java/util/concurrent/ConcurrentLinkedQueue.java index b7beda274da..54bd8cbfff5 100644 --- a/jdk/src/share/classes/java/util/concurrent/ConcurrentLinkedQueue.java +++ b/jdk/src/share/classes/java/util/concurrent/ConcurrentLinkedQueue.java @@ -194,12 +194,22 @@ public class ConcurrentLinkedQueue extends AbstractQueue // Unsafe mechanics - private static final sun.misc.Unsafe UNSAFE = - sun.misc.Unsafe.getUnsafe(); - private static final long nextOffset = - objectFieldOffset(UNSAFE, "next", Node.class); - private static final long itemOffset = - objectFieldOffset(UNSAFE, "item", Node.class); + private static final sun.misc.Unsafe UNSAFE; + private static final long itemOffset; + private static final long nextOffset; + + static { + try { + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = Node.class; + itemOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("item")); + nextOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("next")); + } catch (Exception e) { + throw new Error(e); + } + } } /** @@ -790,14 +800,6 @@ public class ConcurrentLinkedQueue extends AbstractQueue throw new NullPointerException(); } - // Unsafe mechanics - - private static final sun.misc.Unsafe UNSAFE = sun.misc.Unsafe.getUnsafe(); - private static final long headOffset = - objectFieldOffset(UNSAFE, "head", ConcurrentLinkedQueue.class); - private static final long tailOffset = - objectFieldOffset(UNSAFE, "tail", ConcurrentLinkedQueue.class); - private boolean casTail(Node cmp, Node val) { return UNSAFE.compareAndSwapObject(this, tailOffset, cmp, val); } @@ -806,15 +808,21 @@ public class ConcurrentLinkedQueue extends AbstractQueue return UNSAFE.compareAndSwapObject(this, headOffset, cmp, val); } - static long objectFieldOffset(sun.misc.Unsafe UNSAFE, - String field, Class klazz) { + // Unsafe mechanics + + private static final sun.misc.Unsafe UNSAFE; + private static final long headOffset; + private static final long tailOffset; + static { try { - return UNSAFE.objectFieldOffset(klazz.getDeclaredField(field)); - } catch (NoSuchFieldException e) { - // Convert Exception to corresponding Error - NoSuchFieldError error = new NoSuchFieldError(field); - error.initCause(e); - throw error; + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = ConcurrentLinkedQueue.class; + headOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("head")); + tailOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("tail")); + } catch (Exception e) { + throw new Error(e); } } } diff --git a/jdk/src/share/classes/java/util/concurrent/ConcurrentSkipListMap.java b/jdk/src/share/classes/java/util/concurrent/ConcurrentSkipListMap.java index 64d28cf6040..f1b11140291 100644 --- a/jdk/src/share/classes/java/util/concurrent/ConcurrentSkipListMap.java +++ b/jdk/src/share/classes/java/util/concurrent/ConcurrentSkipListMap.java @@ -507,13 +507,24 @@ public class ConcurrentSkipListMap extends AbstractMap return new AbstractMap.SimpleImmutableEntry(key, v); } - // Unsafe mechanics - private static final sun.misc.Unsafe UNSAFE = sun.misc.Unsafe.getUnsafe(); - private static final long valueOffset = - objectFieldOffset(UNSAFE, "value", Node.class); - private static final long nextOffset = - objectFieldOffset(UNSAFE, "next", Node.class); + // UNSAFE mechanics + private static final sun.misc.Unsafe UNSAFE; + private static final long valueOffset; + private static final long nextOffset; + + static { + try { + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = Node.class; + valueOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("value")); + nextOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("next")); + } catch (Exception e) { + throw new Error(e); + } + } } /* ---------------- Indexing -------------- */ @@ -580,10 +591,18 @@ public class ConcurrentSkipListMap extends AbstractMap } // Unsafe mechanics - private static final sun.misc.Unsafe UNSAFE = sun.misc.Unsafe.getUnsafe(); - private static final long rightOffset = - objectFieldOffset(UNSAFE, "right", Index.class); - + private static final sun.misc.Unsafe UNSAFE; + private static final long rightOffset; + static { + try { + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = Index.class; + rightOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("right")); + } catch (Exception e) { + throw new Error(e); + } + } } /* ---------------- Head nodes -------------- */ @@ -3082,20 +3101,16 @@ public class ConcurrentSkipListMap extends AbstractMap } // Unsafe mechanics - private static final sun.misc.Unsafe UNSAFE = sun.misc.Unsafe.getUnsafe(); - private static final long headOffset = - objectFieldOffset(UNSAFE, "head", ConcurrentSkipListMap.class); - - static long objectFieldOffset(sun.misc.Unsafe UNSAFE, - String field, Class klazz) { + private static final sun.misc.Unsafe UNSAFE; + private static final long headOffset; + static { try { - return UNSAFE.objectFieldOffset(klazz.getDeclaredField(field)); - } catch (NoSuchFieldException e) { - // Convert Exception to corresponding Error - NoSuchFieldError error = new NoSuchFieldError(field); - error.initCause(e); - throw error; + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = ConcurrentSkipListMap.class; + headOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("head")); + } catch (Exception e) { + throw new Error(e); } } - } diff --git a/jdk/src/share/classes/java/util/concurrent/ConcurrentSkipListSet.java b/jdk/src/share/classes/java/util/concurrent/ConcurrentSkipListSet.java index 09f8dc6f5b2..04b845c3f87 100644 --- a/jdk/src/share/classes/java/util/concurrent/ConcurrentSkipListSet.java +++ b/jdk/src/share/classes/java/util/concurrent/ConcurrentSkipListSet.java @@ -470,16 +470,20 @@ public class ConcurrentSkipListSet } // Support for resetting map in clone - private static final Unsafe unsafe = Unsafe.getUnsafe(); + private void setMap(ConcurrentNavigableMap map) { + UNSAFE.putObjectVolatile(this, mapOffset, map); + } + + private static final sun.misc.Unsafe UNSAFE; private static final long mapOffset; static { try { - mapOffset = unsafe.objectFieldOffset - (ConcurrentSkipListSet.class.getDeclaredField("m")); - } catch (Exception ex) { throw new Error(ex); } + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = ConcurrentSkipListSet.class; + mapOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("m")); + } catch (Exception e) { + throw new Error(e); + } } - private void setMap(ConcurrentNavigableMap map) { - unsafe.putObjectVolatile(this, mapOffset, map); - } - } diff --git a/jdk/src/share/classes/java/util/concurrent/CopyOnWriteArrayList.java b/jdk/src/share/classes/java/util/concurrent/CopyOnWriteArrayList.java index 8c5193538fa..e39cbeeb2d7 100644 --- a/jdk/src/share/classes/java/util/concurrent/CopyOnWriteArrayList.java +++ b/jdk/src/share/classes/java/util/concurrent/CopyOnWriteArrayList.java @@ -1318,16 +1318,19 @@ public class CopyOnWriteArrayList } // Support for resetting lock while deserializing - private static final Unsafe unsafe = Unsafe.getUnsafe(); + private void resetLock() { + UNSAFE.putObjectVolatile(this, lockOffset, new ReentrantLock()); + } + private static final sun.misc.Unsafe UNSAFE; private static final long lockOffset; static { try { - lockOffset = unsafe.objectFieldOffset - (CopyOnWriteArrayList.class.getDeclaredField("lock")); - } catch (Exception ex) { throw new Error(ex); } + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = CopyOnWriteArrayList.class; + lockOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("lock")); + } catch (Exception e) { + throw new Error(e); + } } - private void resetLock() { - unsafe.putObjectVolatile(this, lockOffset, new ReentrantLock()); - } - } diff --git a/jdk/src/share/classes/java/util/concurrent/LinkedTransferQueue.java b/jdk/src/share/classes/java/util/concurrent/LinkedTransferQueue.java index fec92a8b189..7571978169b 100644 --- a/jdk/src/share/classes/java/util/concurrent/LinkedTransferQueue.java +++ b/jdk/src/share/classes/java/util/concurrent/LinkedTransferQueue.java @@ -525,16 +525,27 @@ public class LinkedTransferQueue extends AbstractQueue return false; } - // Unsafe mechanics - private static final sun.misc.Unsafe UNSAFE = sun.misc.Unsafe.getUnsafe(); - private static final long nextOffset = - objectFieldOffset(UNSAFE, "next", Node.class); - private static final long itemOffset = - objectFieldOffset(UNSAFE, "item", Node.class); - private static final long waiterOffset = - objectFieldOffset(UNSAFE, "waiter", Node.class); - private static final long serialVersionUID = -3375979862319811754L; + + // Unsafe mechanics + private static final sun.misc.Unsafe UNSAFE; + private static final long itemOffset; + private static final long nextOffset; + private static final long waiterOffset; + static { + try { + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = Node.class; + itemOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("item")); + nextOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("next")); + waiterOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("waiter")); + } catch (Exception e) { + throw new Error(e); + } + } } /** head of the queue; null until first enqueue */ @@ -1312,23 +1323,22 @@ public class LinkedTransferQueue extends AbstractQueue // Unsafe mechanics - private static final sun.misc.Unsafe UNSAFE = sun.misc.Unsafe.getUnsafe(); - private static final long headOffset = - objectFieldOffset(UNSAFE, "head", LinkedTransferQueue.class); - private static final long tailOffset = - objectFieldOffset(UNSAFE, "tail", LinkedTransferQueue.class); - private static final long sweepVotesOffset = - objectFieldOffset(UNSAFE, "sweepVotes", LinkedTransferQueue.class); - - static long objectFieldOffset(sun.misc.Unsafe UNSAFE, - String field, Class klazz) { + private static final sun.misc.Unsafe UNSAFE; + private static final long headOffset; + private static final long tailOffset; + private static final long sweepVotesOffset; + static { try { - return UNSAFE.objectFieldOffset(klazz.getDeclaredField(field)); - } catch (NoSuchFieldException e) { - // Convert Exception to corresponding Error - NoSuchFieldError error = new NoSuchFieldError(field); - error.initCause(e); - throw error; + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = LinkedTransferQueue.class; + headOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("head")); + tailOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("tail")); + sweepVotesOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("sweepVotes")); + } catch (Exception e) { + throw new Error(e); } } } diff --git a/jdk/src/share/classes/java/util/concurrent/Phaser.java b/jdk/src/share/classes/java/util/concurrent/Phaser.java index d5725b3e1a2..7e362219179 100644 --- a/jdk/src/share/classes/java/util/concurrent/Phaser.java +++ b/jdk/src/share/classes/java/util/concurrent/Phaser.java @@ -1137,18 +1137,16 @@ public class Phaser { // Unsafe mechanics - private static final sun.misc.Unsafe UNSAFE = sun.misc.Unsafe.getUnsafe(); - private static final long stateOffset = - objectFieldOffset("state", Phaser.class); - - private static long objectFieldOffset(String field, Class klazz) { + private static final sun.misc.Unsafe UNSAFE; + private static final long stateOffset; + static { try { - return UNSAFE.objectFieldOffset(klazz.getDeclaredField(field)); - } catch (NoSuchFieldException e) { - // Convert Exception to corresponding Error - NoSuchFieldError error = new NoSuchFieldError(field); - error.initCause(e); - throw error; + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = Phaser.class; + stateOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("state")); + } catch (Exception e) { + throw new Error(e); } } } diff --git a/jdk/src/share/classes/java/util/concurrent/PriorityBlockingQueue.java b/jdk/src/share/classes/java/util/concurrent/PriorityBlockingQueue.java index 60928ac8a74..5ad7fa71d71 100644 --- a/jdk/src/share/classes/java/util/concurrent/PriorityBlockingQueue.java +++ b/jdk/src/share/classes/java/util/concurrent/PriorityBlockingQueue.java @@ -963,21 +963,16 @@ public class PriorityBlockingQueue extends AbstractQueue } // Unsafe mechanics - private static final sun.misc.Unsafe UNSAFE = sun.misc.Unsafe.getUnsafe(); - private static final long allocationSpinLockOffset = - objectFieldOffset(UNSAFE, "allocationSpinLock", - PriorityBlockingQueue.class); - - static long objectFieldOffset(sun.misc.Unsafe UNSAFE, - String field, Class klazz) { + private static final sun.misc.Unsafe UNSAFE; + private static final long allocationSpinLockOffset; + static { try { - return UNSAFE.objectFieldOffset(klazz.getDeclaredField(field)); - } catch (NoSuchFieldException e) { - // Convert Exception to corresponding Error - NoSuchFieldError error = new NoSuchFieldError(field); - error.initCause(e); - throw error; + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = PriorityBlockingQueue.class; + allocationSpinLockOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("allocationSpinLock")); + } catch (Exception e) { + throw new Error(e); } } - } diff --git a/jdk/src/share/classes/java/util/concurrent/SynchronousQueue.java b/jdk/src/share/classes/java/util/concurrent/SynchronousQueue.java index 47b352f9e35..d02f56d9b72 100644 --- a/jdk/src/share/classes/java/util/concurrent/SynchronousQueue.java +++ b/jdk/src/share/classes/java/util/concurrent/SynchronousQueue.java @@ -279,12 +279,22 @@ public class SynchronousQueue extends AbstractQueue } // Unsafe mechanics - private static final sun.misc.Unsafe UNSAFE = sun.misc.Unsafe.getUnsafe(); - private static final long nextOffset = - objectFieldOffset(UNSAFE, "next", SNode.class); - private static final long matchOffset = - objectFieldOffset(UNSAFE, "match", SNode.class); + private static final sun.misc.Unsafe UNSAFE; + private static final long matchOffset; + private static final long nextOffset; + static { + try { + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = SNode.class; + matchOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("match")); + nextOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("next")); + } catch (Exception e) { + throw new Error(e); + } + } } /** The head (top) of the stack */ @@ -498,10 +508,18 @@ public class SynchronousQueue extends AbstractQueue } // Unsafe mechanics - private static final sun.misc.Unsafe UNSAFE = sun.misc.Unsafe.getUnsafe(); - private static final long headOffset = - objectFieldOffset(UNSAFE, "head", TransferStack.class); - + private static final sun.misc.Unsafe UNSAFE; + private static final long headOffset; + static { + try { + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = TransferStack.class; + headOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("head")); + } catch (Exception e) { + throw new Error(e); + } + } } /** Dual Queue */ @@ -558,11 +576,22 @@ public class SynchronousQueue extends AbstractQueue } // Unsafe mechanics - private static final sun.misc.Unsafe UNSAFE = sun.misc.Unsafe.getUnsafe(); - private static final long nextOffset = - objectFieldOffset(UNSAFE, "next", QNode.class); - private static final long itemOffset = - objectFieldOffset(UNSAFE, "item", QNode.class); + private static final sun.misc.Unsafe UNSAFE; + private static final long itemOffset; + private static final long nextOffset; + + static { + try { + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = QNode.class; + itemOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("item")); + nextOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("next")); + } catch (Exception e) { + throw new Error(e); + } + } } /** Head of queue */ @@ -791,15 +820,24 @@ public class SynchronousQueue extends AbstractQueue } } - // unsafe mechanics - private static final sun.misc.Unsafe UNSAFE = sun.misc.Unsafe.getUnsafe(); - private static final long headOffset = - objectFieldOffset(UNSAFE, "head", TransferQueue.class); - private static final long tailOffset = - objectFieldOffset(UNSAFE, "tail", TransferQueue.class); - private static final long cleanMeOffset = - objectFieldOffset(UNSAFE, "cleanMe", TransferQueue.class); - + private static final sun.misc.Unsafe UNSAFE; + private static final long headOffset; + private static final long tailOffset; + private static final long cleanMeOffset; + static { + try { + UNSAFE = sun.misc.Unsafe.getUnsafe(); + Class k = TransferQueue.class; + headOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("head")); + tailOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("tail")); + cleanMeOffset = UNSAFE.objectFieldOffset + (k.getDeclaredField("cleanMe")); + } catch (Exception e) { + throw new Error(e); + } + } } /**