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

[David Holmes]

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
>