[Guile-commits] 17/17: Deprecate critical sections

[Andy Wingo]

wingo pushed a commit to branch master
in repository guile.

commit fcc6a7ba20d08f81602b395c04e04f9acc8403e3
Author: Andy Wingo <address@hidden>
Date:   Tue Nov 1 23:48:42 2016 +0100

    Deprecate critical sections
    
    * NEWS: Deprecate critical sections.
    * doc/ref/api-scheduling.texi (Critical Sections): Remove.
    * libguile/async.c:
    * libguile/async.h:
    * libguile/deprecated.c:
    * libguile/deprecated.h:
    * libguile/threads.c:
    * libguile/threads.h: Deprecate critical section API.
---
 NEWS                        |    8 ++++++++
 doc/ref/api-scheduling.texi |   46 -------------------------------------------
 libguile/async.c            |   20 -------------------
 libguile/async.h            |    8 --------
 libguile/deprecated.c       |   41 ++++++++++++++++++++++++++++++++++++++
 libguile/deprecated.h       |    9 +++++++++
 libguile/threads.c          |   13 ------------
 libguile/threads.h          |    2 --
 8 files changed, 58 insertions(+), 89 deletions(-)

diff --git a/NEWS b/NEWS
index a5b1a01..7233d2f 100644
--- a/NEWS
+++ b/NEWS
@@ -43,6 +43,14 @@ trivial unused data structure.  Now that we have deprecated 
the old
 "user async" facility, we have been able to clarify our documentation to
 only refer to "asyncs".
 
+** Critical sections deprecated
+
+Critical sections have long been just a fancy way to lock a mutex and
+defer asynchronous interrupts.  Instead of SCM_CRITICAL_SECTION_START,
+make sure you're in a "scm_dynwind_begin (0)" and use
+scm_dynwind_pthread_mutex_lock instead, possibly also with
+scm_dynwind_block_asyncs.
+
 * Bug fixes
 ** cancel-thread uses asynchronous interrupts, not pthread_cancel
 
diff --git a/doc/ref/api-scheduling.texi b/doc/ref/api-scheduling.texi
index c7c7c8e..d563304 100644
--- a/doc/ref/api-scheduling.texi
+++ b/doc/ref/api-scheduling.texi
@@ -13,7 +13,6 @@
 * Atomics::                     Atomic references.
 * Mutexes and Condition Variables:: Synchronization primitives.
 * Blocking::                    How to block properly in guile mode.
-* Critical Sections::           Avoiding concurrency and reentries.
 * Fluids and Dynamic States::   Thread-local variables, etc.
 * Parameters::                  Dynamic scoping in Scheme.
 * Futures::                     Fine-grain parallelism.
@@ -619,51 +618,6 @@ delivery of an async causes this function to be 
interrupted.
 @end deftypefn
 
 
address@hidden Critical Sections
address@hidden Critical Sections
-
address@hidden  {C Macro} SCM_CRITICAL_SECTION_START
address@hidden {C Macro} SCM_CRITICAL_SECTION_END
-These two macros can be used to delimit a critical section.
-Syntactically, they are both statements and need to be followed
-immediately by a semicolon.
-
-Executing @code{SCM_CRITICAL_SECTION_START} will lock a recursive mutex
-and block the executing of asyncs.  Executing
address@hidden will unblock the execution of system
-asyncs and unlock the mutex.  Thus, the code that executes between these
-two macros can only be executed in one thread at any one time and no
-asyncs will run.  However, because the mutex is a recursive one, the
-code might still be reentered by the same thread.  You must either allow
-for this or avoid it, both by careful coding.
-
-On the other hand, critical sections delimited with these macros can
-be nested since the mutex is recursive.
-
-You must make sure that for each @code{SCM_CRITICAL_SECTION_START},
-the corresponding @code{SCM_CRITICAL_SECTION_END} is always executed.
-This means that no non-local exit (such as a signalled error) might
-happen, for example.
address@hidden deffn
-
address@hidden {C Function} void scm_dynwind_critical_section (SCM mutex)
-Call @code{scm_dynwind_lock_mutex} on @var{mutex} and call
address@hidden  When @var{mutex} is false, a recursive
-mutex provided by Guile is used instead.
-
-The effect of a call to @code{scm_dynwind_critical_section} is that
-the current dynwind context (@pxref{Dynamic Wind}) turns into a
-critical section.  Because of the locked mutex, no second thread can
-enter it concurrently and because of the blocked asyncs, no system
-async can reenter it from the current thread.
-
-When the current thread reenters the critical section anyway, the kind
-of @var{mutex} determines what happens: When @var{mutex} is recursive,
-the reentry is allowed.  When it is a normal mutex, an error is
-signalled.
address@hidden deftypefn
-
-
 @node Fluids and Dynamic States
 @subsection Fluids and Dynamic States
 
diff --git a/libguile/async.c b/libguile/async.c
index e456167..b4a2c2a 100644
--- a/libguile/async.c
+++ b/libguile/async.c
@@ -299,30 +299,10 @@ scm_c_call_with_unblocked_asyncs (void *(*proc) (void 
*data), void *data)
 }
 
 
-static scm_i_pthread_mutex_t critical_section_mutex;
-
-void
-scm_critical_section_start (void)
-{
-  scm_i_pthread_mutex_lock (&critical_section_mutex);
-  SCM_I_CURRENT_THREAD->block_asyncs++;
-}
-
-void
-scm_critical_section_end (void)
-{
-  SCM_I_CURRENT_THREAD->block_asyncs--;
-  scm_i_pthread_mutex_unlock (&critical_section_mutex);
-  scm_async_tick ();
-}
-
-
 
 void
 scm_init_async ()
 {
-  scm_i_pthread_mutex_init (&critical_section_mutex,
-                           scm_i_pthread_mutexattr_recursive);
 #include "libguile/async.x"
 }
 
diff --git a/libguile/async.h b/libguile/async.h
index b709894..2a57236 100644
--- a/libguile/async.h
+++ b/libguile/async.h
@@ -46,14 +46,6 @@ SCM_API void *scm_c_call_with_unblocked_asyncs (void *(*p) 
(void *d), void *d);
 SCM_API void scm_dynwind_block_asyncs (void);
 SCM_API void scm_dynwind_unblock_asyncs (void);
 
-/* Critical sections */
-
-SCM_API void scm_critical_section_start (void);
-SCM_API void scm_critical_section_end (void);
-
-#define SCM_CRITICAL_SECTION_START  scm_critical_section_start ()
-#define SCM_CRITICAL_SECTION_END    scm_critical_section_end ()
-
 SCM_INTERNAL void scm_init_async (void);
 
 #endif  /* SCM_ASYNC_H */
diff --git a/libguile/deprecated.c b/libguile/deprecated.c
index 228c5d8..c8d353f 100644
--- a/libguile/deprecated.c
+++ b/libguile/deprecated.c
@@ -639,6 +639,44 @@ SCM_DEFINE (scm_run_asyncs, "run-asyncs", 1, 0, 0,
 }
 #undef FUNC_NAME
 
+
+static scm_i_pthread_mutex_t critical_section_mutex;
+static SCM dynwind_critical_section_mutex;
+
+void
+scm_critical_section_start (void)
+{
+  scm_c_issue_deprecation_warning
+    ("Critical sections are deprecated.  Instead use dynwinds and "
+     "\"scm_dynwind_pthread_mutex_lock\" together with "
+     "\"scm_dynwind_block_asyncs\" if appropriate.");
+
+  scm_i_pthread_mutex_lock (&critical_section_mutex);
+  SCM_I_CURRENT_THREAD->block_asyncs++;
+}
+
+void
+scm_critical_section_end (void)
+{
+  SCM_I_CURRENT_THREAD->block_asyncs--;
+  scm_i_pthread_mutex_unlock (&critical_section_mutex);
+  scm_async_tick ();
+}
+
+void
+scm_dynwind_critical_section (SCM mutex)
+{
+  scm_c_issue_deprecation_warning
+    ("Critical sections are deprecated.  Instead use dynwinds and "
+     "\"scm_dynwind_pthread_mutex_lock\" together with "
+     "\"scm_dynwind_block_asyncs\" if appropriate.");
+
+  if (scm_is_false (mutex))
+    mutex = dynwind_critical_section_mutex;
+  scm_dynwind_lock_mutex (mutex);
+  scm_dynwind_block_asyncs ();
+}
+
 
 
 
@@ -648,6 +686,9 @@ scm_i_init_deprecated ()
   scm_tc16_arbiter = scm_make_smob_type ("arbiter", 0);
   scm_set_smob_print (scm_tc16_arbiter, arbiter_print);
   tc16_async = scm_make_smob_type ("async", 0);
+  scm_i_pthread_mutex_init (&critical_section_mutex,
+                           scm_i_pthread_mutexattr_recursive);
+  dynwind_critical_section_mutex = scm_make_recursive_mutex ();
 #include "libguile/deprecated.x"
 }
 
diff --git a/libguile/deprecated.h b/libguile/deprecated.h
index 7eb7ee4..d8ce816 100644
--- a/libguile/deprecated.h
+++ b/libguile/deprecated.h
@@ -229,6 +229,15 @@ SCM_DEPRECATED SCM scm_run_asyncs (SCM list_of_a);
 
 
 
+SCM_DEPRECATED void scm_critical_section_start (void);
+SCM_DEPRECATED void scm_critical_section_end (void);
+SCM_DEPRECATED void scm_dynwind_critical_section (SCM mutex);
+
+#define SCM_CRITICAL_SECTION_START  scm_critical_section_start ()
+#define SCM_CRITICAL_SECTION_END    scm_critical_section_end ()
+
+
+
 void scm_i_init_deprecated (void);
 
 #endif
diff --git a/libguile/threads.c b/libguile/threads.c
index c1f4169..43bd313 100644
--- a/libguile/threads.c
+++ b/libguile/threads.c
@@ -1938,17 +1938,6 @@ static scm_i_pthread_cond_t wake_up_cond;
 static int threads_initialized_p = 0;
 
 
-static SCM dynwind_critical_section_mutex;
-
-void
-scm_dynwind_critical_section (SCM mutex)
-{
-  if (scm_is_false (mutex))
-    mutex = dynwind_critical_section_mutex;
-  scm_dynwind_lock_mutex (mutex);
-  scm_dynwind_block_asyncs ();
-}
-
 /*** Initialization */
 
 scm_i_pthread_mutex_t scm_i_misc_mutex;
@@ -2011,8 +2000,6 @@ scm_init_threads ()
   guilify_self_2 (SCM_BOOL_F);
   threads_initialized_p = 1;
 
-  dynwind_critical_section_mutex = scm_make_recursive_mutex ();
-
   scm_c_register_extension ("libguile-" SCM_EFFECTIVE_VERSION,
                             "scm_init_ice_9_threads",
                             scm_init_ice_9_threads, NULL);
diff --git a/libguile/threads.h b/libguile/threads.h
index 6cf575b..f6165a5 100644
--- a/libguile/threads.h
+++ b/libguile/threads.h
@@ -178,8 +178,6 @@ SCM_API SCM scm_all_threads (void);
 SCM_API int scm_c_thread_exited_p (SCM thread);
 SCM_API SCM scm_thread_exited_p (SCM thread);
 
-SCM_API void scm_dynwind_critical_section (SCM mutex);
-
 #ifdef BUILDING_LIBGUILE
 
 /* Though we don't need the key for SCM_I_CURRENT_THREAD if we have TLS,