Unsafe.park/unpark, j.u.c.LockSupport and Thread

[David Holmes]

Oh I just realized/remembered why we use Unsafe for this - it is because 
of the potential for intrinsics.

David

On 10/02/2015 10:02 PM, David Holmes wrote:
> Hi Paul,
>
> When we added j.u.c there was reluctance to add these methods to Thread
> - this was the normal policy (for the time) of don't touch the core
> classes. So LockSupport is the public API and Unsafe was chosen as the
> implementation as it is a de-facto interface to the VM from JDK code
> rather then defining explicit native methods (I must admit I'm unsure
> why we went this route rather than simply hooking into jvm.cpp with
> JVM_park/JVM_unpark. I think in many ways it was/is just easier to call
> an Unsafe method and define a new unsafe.cpp native call. Plus we had a
> bunch of other Unsafe API's being used from j.u.c.)
>
> So we can't just move things from LockSupport to Thread as we'd need to
> deprecate first etc etc. But I don't see any reason why we couldn't move
> the implementation from unsafe.cpp to jvm.cpp and invoke via a native
> method implementation of LockSupport. It would still be just as "unsafe"
> of course.
>
> Aside: this is where the infamous "spurious wakeup" from park() can
> arise. If you just randomly unpark() a Thread there is nothing to
> guarantee that the thread doesn't terminate and free its native
> resources while you are working on it. But the ParkEvents are
> type-stable-memory, so even if the event has been disassociated from the
> target thread you can still call unpark() as it is never freed. However
> if that ParkEvent has since been reused by another thread, then that
> thread will encounter a "spurious wakeup" if it calls park().
>
> Cheers,
> David
>
> On 10/02/2015 8:49 PM, Paul Sandoz wrote:
>> Hi,
>>
>> As part of the effort around Unsafe (some may have noticed some
>> cleanup work) i have been recently looking at the park/unpark methods.
>>
>> The class java.util.concurrent.locks.LockSupport [1] has some thin
>> public wrappers around these methods:
>>
>>      public static void unpark(Thread thread) {
>>          if (thread != null)
>>              U.unpark(thread);
>>      }
>>
>>      public static void park() {
>>          U.park(false, 0L);
>>      }
>>
>>      public static void parkNanos(long nanos) {
>>          if (nanos > 0)
>>              U.park(false, nanos);
>>      }
>>
>>      public static void parkUntil(long deadline) {
>>          U.park(true, deadline);
>>      }
>>
>> Is not clear to me what is exactly unsafe about park/unpark and why
>> they were not originally placed on Thread itself given the above
>> wrapping.
>>
>> There is mention of unpark being unsafe with regards to native code
>> and ensuring the thread has not been destroyed:
>>
>>    /**
>>     * Unblock the given thread blocked on <tt>park</tt>, or, if it is
>>     * not blocked, cause the subsequent call to <tt>park</tt> not to
>>     * block.  Note: this operation is "unsafe" solely because the
>>     * caller must somehow ensure that the thread has not been
>>     * destroyed. Nothing special is usually required to ensure this
>>     * when called from Java (in which there will ordinarily be a live
>>     * reference to the thread) but this is not nearly-automatically
>>     * so when calling from native code.
>>     * @param thread the thread to unpark.
>>     *
>>     */
>>    public native void unpark(Object thread);
>>
>> However, native code is anyway inherently unsafe.
>>
>>
>> In addition this class has a cosy relationship with Thread (it wants
>> to poke into Thread's non-public fields):
>>
>>      // Hotspot implementation via intrinsics API
>>      private static final sun.misc.Unsafe U =
>> sun.misc.Unsafe.getUnsafe();
>>      private static final long PARKBLOCKER;
>>      private static final long SEED;
>>      private static final long PROBE;
>>      private static final long SECONDARY;
>>      static {
>>          try {
>>              PARKBLOCKER = U.objectFieldOffset
>>                  (Thread.class.getDeclaredField("parkBlocker"));
>>              SEED = U.objectFieldOffset
>>
>> (Thread.class.getDeclaredField("threadLocalRandomSeed"));
>>              PROBE = U.objectFieldOffset
>>
>> (Thread.class.getDeclaredField("threadLocalRandomProbe"));
>>              SECONDARY = U.objectFieldOffset
>>
>> (Thread.class.getDeclaredField("threadLocalRandomSecondarySeed"));
>>          } catch (ReflectiveOperationException e) {
>>              throw new Error(e);
>>          }
>>      }
>>
>> Although only PARKBLOCKER and SECONDARY are used AFAICT.
>>
>> I am sure there is some history behind all this... but in my ignorance
>> of the past perhaps it's time to reconsider?
>>
>> We could reduce the coupling on Thread and dependency on Unsafe if we
>> consider moving park/unpark and LockSupport functionality to Thread
>> itself.
>>
>> Thoughts?
>>
>> Paul.
>>
>> [1]
>> http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/main/java/util/concurrent/locks/LockSupport.java?view=co
>>
>>