Remove criticalRegionLock

1. It is unused.
2. It doesn't provide the guarantee one think it does. See https://issues.dlang.org/show_bug.cgi?id=24741

changelog/druntime.criticalRegionLock.dd

@@ -0,0 +1,5 @@
+Remove criticalRegionLock
+
+The criticalRegionLock feature suffer from a serious design flaw: https://issues.dlang.org/show_bug.cgi?id=24741
+
+It turns out it is not used, so rather than fixing the flaw, the feature was removed.

druntime/src/core/thread/osthread.d

@@ -1088,52 +1088,6 @@ unittest
     t.join();
 }
 
-unittest
-{
-    // NOTE: This entire test is based on the assumption that no
-    //       memory is allocated after the child thread is
-    //       started. If an allocation happens, a collection could
-    //       trigger, which would cause the synchronization below
-    //       to cause a deadlock.
-    // NOTE: DO NOT USE LOCKS IN CRITICAL REGIONS IN NORMAL CODE.
-
-    import core.sync.semaphore;
-
-    auto sema = new Semaphore(),
-         semb = new Semaphore();
-
-    auto thr = new Thread(
-    {
-        thread_enterCriticalRegion();
-        assert(thread_inCriticalRegion());
-        sema.notify();
-
-        semb.wait();
-        assert(thread_inCriticalRegion());
-
-        thread_exitCriticalRegion();
-        assert(!thread_inCriticalRegion());
-        sema.notify();
-
-        semb.wait();
-        assert(!thread_inCriticalRegion());
-    });
-
-    thr.start();
-
-    sema.wait();
-    synchronized (ThreadBase.criticalRegionLock)
-        assert(thr.m_isInCriticalRegion);
-    semb.notify();
-
-    sema.wait();
-    synchronized (ThreadBase.criticalRegionLock)
-        assert(!thr.m_isInCriticalRegion);
-    semb.notify();
-
-    thr.join();
-}
-
 // https://issues.dlang.org/show_bug.cgi?id=22124
 unittest
 {
@@ -1146,36 +1100,6 @@ unittest
     static assert(!__traits(compiles, () @nogc => fun(thread, 3) ));
 }
 
-unittest
-{
-    import core.sync.semaphore;
-
-    shared bool inCriticalRegion;
-    auto sema = new Semaphore(),
-         semb = new Semaphore();
-
-    auto thr = new Thread(
-    {
-        thread_enterCriticalRegion();
-        inCriticalRegion = true;
-        sema.notify();
-        semb.wait();
-
-        Thread.sleep(dur!"msecs"(1));
-        inCriticalRegion = false;
-        thread_exitCriticalRegion();
-    });
-    thr.start();
-
-    sema.wait();
-    assert(inCriticalRegion);
-    semb.notify();
-
-    thread_suspendAll();
-    assert(!inCriticalRegion);
-    thread_resumeAll();
-}
-
 @nogc @safe nothrow
 unittest
 {
@@ -1562,21 +1486,11 @@ private extern(D) void* getStackBottom() nothrow @nogc
  */
 private extern (D) bool suspend( Thread t ) nothrow @nogc
 {
-    Duration waittime = dur!"usecs"(10);
- Lagain:
     if (!t.isRunning)
     {
         Thread.remove(t);
         return false;
     }
-    else if (t.m_isInCriticalRegion)
-    {
-        Thread.criticalRegionLock.unlock_nothrow();
-        Thread.sleep(waittime);
-        if (waittime < dur!"msecs"(10)) waittime *= 2;
-        Thread.criticalRegionLock.lock_nothrow();
-        goto Lagain;
-    }
 
     version (Windows)
     {
@@ -1821,8 +1735,6 @@ extern (C) void thread_suspendAll() nothrow
         if ( ++suspendDepth > 1 )
             return;
 
-        Thread.criticalRegionLock.lock_nothrow();
-        scope (exit) Thread.criticalRegionLock.unlock_nothrow();
         size_t cnt;
         bool suspendedSelf;
         Thread t = ThreadBase.sm_tbeg.toThread;

druntime/src/core/thread/threadbase.d

@@ -452,7 +452,6 @@ package:
     string              m_name;
     size_t              m_sz;
     bool                m_isDaemon;
-    bool                m_isInCriticalRegion;
     Throwable           m_unhandled;
 
     ///////////////////////////////////////////////////////////////////////////
@@ -560,25 +559,17 @@ package(core.thread):
         return cast(Mutex)_slock.ptr;
     }
 
-    @property static Mutex criticalRegionLock() nothrow @nogc
-    {
-        return cast(Mutex)_criticalRegionLock.ptr;
-    }
-
     __gshared align(mutexAlign) void[mutexClassInstanceSize] _slock;
-    __gshared align(mutexAlign) void[mutexClassInstanceSize] _criticalRegionLock;
 
     static void initLocks() @nogc nothrow
     {
         import core.lifetime : emplace;
         emplace!Mutex(_slock[]);
-        emplace!Mutex(_criticalRegionLock[]);
     }
 
     static void termLocks() @nogc nothrow
     {
         (cast(Mutex)_slock.ptr).__dtor();
-        (cast(Mutex)_criticalRegionLock.ptr).__dtor();
     }
 
     __gshared StackContext*  sm_cbeg;
@@ -1140,75 +1131,6 @@ extern (C) void thread_scanAll(scope ScanAllThreadsFn scan) nothrow
 
 private alias thread_yield = externDFunc!("core.thread.osthread.thread_yield", void function() @nogc nothrow);
 
-/**
- * Signals that the code following this call is a critical region. Any code in
- * this region must finish running before the calling thread can be suspended
- * by a call to thread_suspendAll.
- *
- * This function is, in particular, meant to help maintain garbage collector
- * invariants when a lock is not used.
- *
- * A critical region is exited with thread_exitCriticalRegion.
- *
- * $(RED Warning):
- * Using critical regions is extremely error-prone. For instance, using locks
- * inside a critical region can easily result in a deadlock when another thread
- * holding the lock already got suspended.
- *
- * The term and concept of a 'critical region' comes from
- * $(LINK2 https://github.com/mono/mono/blob/521f4a198e442573c400835ef19bbb36b60b0ebb/mono/metadata/sgen-gc.h#L925, Mono's SGen garbage collector).
- *
- * In:
- *  The calling thread must be attached to the runtime.
- */
-extern (C) void thread_enterCriticalRegion() @nogc
-in
-{
-    assert(ThreadBase.getThis());
-}
-do
-{
-    synchronized (ThreadBase.criticalRegionLock)
-        ThreadBase.getThis().m_isInCriticalRegion = true;
-}
-
-
-/**
- * Signals that the calling thread is no longer in a critical region. Following
- * a call to this function, the thread can once again be suspended.
- *
- * In:
- *  The calling thread must be attached to the runtime.
- */
-extern (C) void thread_exitCriticalRegion() @nogc
-in
-{
-    assert(ThreadBase.getThis());
-}
-do
-{
-    synchronized (ThreadBase.criticalRegionLock)
-        ThreadBase.getThis().m_isInCriticalRegion = false;
-}
-
-
-/**
- * Returns true if the current thread is in a critical region; otherwise, false.
- *
- * In:
- *  The calling thread must be attached to the runtime.
- */
-extern (C) bool thread_inCriticalRegion() @nogc
-in
-{
-    assert(ThreadBase.getThis());
-}
-do
-{
-    synchronized (ThreadBase.criticalRegionLock)
-        return ThreadBase.getThis().m_isInCriticalRegion;
-}
-
 
 /**
 * A callback for thread errors in D during collections. Since an allocation is not possible
@@ -1229,22 +1151,6 @@ package void onThreadError(string msg) nothrow @nogc
     throw error;
 }
 
-unittest
-{
-    assert(!thread_inCriticalRegion());
-
-    {
-        thread_enterCriticalRegion();
-
-        scope (exit)
-            thread_exitCriticalRegion();
-
-        assert(thread_inCriticalRegion());
-    }
-
-    assert(!thread_inCriticalRegion());
-}
-
 
 /**
  * Indicates whether an address has been marked by the GC.