diff --git a/NEWS b/NEWS index a5b1a01ca..7233d2f3c 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 c7c7c8ebe..d5633044e 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 -@node Critical Sections -@subsection Critical Sections - -@deffn {C Macro} SCM_CRITICAL_SECTION_START -@deffnx {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 -@code{SCM_CRITICAL_SECTION_END} 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. -@end deffn - -@deftypefn {C Function} void scm_dynwind_critical_section (SCM mutex) -Call @code{scm_dynwind_lock_mutex} on @var{mutex} and call -@code{scm_dynwind_block_asyncs}. 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. -@end deftypefn - - @node Fluids and Dynamic States @subsection Fluids and Dynamic States diff --git a/libguile/async.c b/libguile/async.c index e45616755..b4a2c2ad2 100644 --- a/libguile/async.c +++ b/libguile/async.c @@ -298,31 +298,11 @@ scm_c_call_with_unblocked_asyncs (void *(*proc) (void *data), void *data) return ans; } - -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 b709894c8..2a57236ca 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 228c5d83b..c8d353f89 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 7eb7ee479..d8ce8166f 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 c1f416981..43bd31310 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 6cf575be1..f6165a558 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,