Skip to content

Commit

Permalink
Use general locale mutex for numeric operations
Browse files Browse the repository at this point in the history 
This commit removes the separate mutex for locking locale-related
numeric operations on threaded perls; instead using the general locale
one.  The previous commit made that a general semaphore, so now suitable
for use for this purpose as well.

This means that the locale can be locked for the duration of some
sprintf operations, longer than before this commit.  But on most modern
platforms, thread-safe locales cause this lock to expand just to a
no-op; so there is no effect on these.  And on the impacted platforms,
one is not supposed to be using locales and threads in combination, as
races can occur.  This lock is used on those perls to keep Perl's
manipulation of LC_NUMERIC thread-safe.  And for those there is also no
effect, as they already lock around those sprintf's.
  • Loading branch information
@khwilliamson
khwilliamson committed May 9, 2021
1 parent 0378d40 commit 7996b1c
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 102 deletions.
1 change: 0 additions & 1 deletion embedvar.h
View file
Expand Up @@ -189,7 +189,6 @@
#define PL_lastgotoprobe (vTHX->Ilastgotoprobe)
#define PL_laststatval (vTHX->Ilaststatval)
#define PL_laststype (vTHX->Ilaststype)
#define PL_lc_numeric_mutex_depth (vTHX->Ilc_numeric_mutex_depth)
#define PL_locale_mutex_depth (vTHX->Ilocale_mutex_depth)
#define PL_localizing (vTHX->Ilocalizing)
#define PL_localpatches (vTHX->Ilocalpatches)
Expand Down
3 changes: 0 additions & 3 deletions intrpvar.h
View file
Expand Up @@ -370,9 +370,6 @@ PERLVAR(I, in_utf8_turkic_locale, bool)
#if defined(USE_LOCALE) && defined(USE_LOCALE_THREADS)
PERLVARI(I, locale_mutex_depth, int, 0) /* Emulate general semaphore */
#endif
#if defined(USE_ITHREADS) && ! defined(USE_THREAD_SAFE_LOCALE)
PERLVARI(I, lc_numeric_mutex_depth, int, 0) /* Emulate general semaphore */
#endif

#ifdef USE_LOCALE_CTYPE
PERLVAR(I, warn_locale, SV *)
Expand Down
7 changes: 0 additions & 7 deletions makedef.pl
View file
Expand Up @@ -376,8 +376,6 @@ sub readvar {
PL_hints_mutex
PL_locale_mutex
PL_locale_mutex_depth
PL_lc_numeric_mutex
PL_lc_numeric_mutex_depth
PL_my_ctx_mutex
PL_perlio_mutex
PL_stashpad
Expand Down Expand Up @@ -451,11 +449,6 @@ sub readvar {
);
}

if ($define{USE_THREAD_SAFE_LOCALE}) {
++$skip{PL_lc_numeric_mutex};
++$skip{PL_lc_numeric_mutex_depth};
}

unless ($define{'USE_DTRACE'}) {
++$skip{$_} foreach qw(
Perl_dtrace_probe_call
Expand Down
103 changes: 18 additions & 85 deletions perl.h
View file
Expand Up @@ -6720,6 +6720,18 @@ the plain locale pragma without a parameter (S<C<use locale>>) is in effect.
} \
} STMT_END

# ifndef USE_THREAD_SAFE_LOCALE

/* By definition, a thread-unsafe locale means we need a critical
* section. */

# ifdef USE_LOCALE_NUMERIC
# define LC_NUMERIC_LOCK(cond_to_panic_if_already_locked) \
LOCALE_LOCK_(cond_to_panic_if_already_locked)
# define LC_NUMERIC_UNLOCK LOCALE_UNLOCK_
# endif
# endif

# ifndef USE_POSIX_2008_LOCALE
# define LOCALE_TERM_POSIX_2008_ NOOP
# else
Expand All @@ -6736,14 +6748,9 @@ the plain locale pragma without a parameter (S<C<use locale>>) is in effect.
} STMT_END
# endif

# define LOCALE_INIT STMT_START { \
MUTEX_INIT(&PL_locale_mutex); \
LOCALE_INIT_LC_NUMERIC_; \
} STMT_END

# define LOCALE_INIT MUTEX_INIT(&PL_locale_mutex)
# define LOCALE_TERM STMT_START { \
MUTEX_DESTROY(&PL_locale_mutex); \
LOCALE_TERM_LC_NUMERIC_; \
LOCALE_TERM_POSIX_2008_; \
} STMT_END
#endif
Expand All @@ -6762,8 +6769,6 @@ the plain locale pragma without a parameter (S<C<use locale>>) is in effect.
/* The whole expression just above was complemented, so here we have no need
* for thread synchronization, most likely it would be that this isn't a
* threaded build. */
# define LC_NUMERIC_LOCK(cond) NOOP
# define LC_NUMERIC_UNLOCK NOOP
# define LOCALECONV_LOCK NOOP
# define LOCALECONV_UNLOCK NOOP
# define LOCALE_READ_LOCK NOOP
Expand Down Expand Up @@ -6837,13 +6842,6 @@ the plain locale pragma without a parameter (S<C<use locale>>) is in effect.
# define WCTOMB_UNLOCK_ LOCALE_UNLOCK_
# endif
# if defined(USE_THREAD_SAFE_LOCALE)
/* On locale thread-safe systems, we don't need these workarounds */
# define LOCALE_TERM_LC_NUMERIC_ NOOP
# define LOCALE_INIT_LC_NUMERIC_ NOOP
# define LC_NUMERIC_LOCK(cond) NOOP
# define LC_NUMERIC_UNLOCK NOOP
# define LOCALE_INIT_LC_NUMERIC_ NOOP
# define LOCALE_TERM_LC_NUMERIC_ NOOP

/* There may be instance core where we this is invoked yet should do
* nothing. Rather than have #ifdef's around them, define it here */
Expand All @@ -6852,79 +6850,14 @@ the plain locale pragma without a parameter (S<C<use locale>>) is in effect.
# else
# define SETLOCALE_LOCK LOCALE_LOCK_(0)
# define SETLOCALE_UNLOCK LOCALE_UNLOCK_

/* On platforms without per-thread locales, when another thread can switch
* our locale, we need another mutex to create critical sections where we
* want the LC_NUMERIC locale to be locked into either the C (standard)
* locale, or the underlying locale, so that other threads interrupting
* this one don't change it to the wrong state before we've had a chance to
* complete our operation. It can stay locked over an entire printf
* operation, for example. And so is made distinct from the LOCALE_LOCK
* mutex.
*
* This simulates kind of a general semaphore. The current thread will
* lock the mutex if the per-thread variable is zero, and then increments
* that variable. Each corresponding UNLOCK decrements the variable until
* it is 0, at which point it actually unlocks the mutex. Since the
* variable is per-thread, there is no race with other threads.
*
* The single argument is a condition to test for, and if true, to panic,
* as this would be an attempt to complement the LC_NUMERIC state, and
* we're not supposed to because it's locked.
*
* Clang improperly gives warnings for this, if not silenced:
* https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#conditional-locks
*
* If LC_NUMERIC_LOCK is combined with one of the LOCKs above, calls to
* that and its corresponding unlock should be contained entirely within
* the locked portion of LC_NUMERIC. Those mutexes should be used only in
* very short sections of code, while LC_NUMERIC_LOCK may span more
* operations. By always following this convention, deadlock should be
* impossible. But if necessary, the two mutexes could be combined. */
# define LC_NUMERIC_LOCK(cond_to_panic_if_already_locked) \
CLANG_DIAG_IGNORE(-Wthread-safety) \
STMT_START { \
if (PL_lc_numeric_mutex_depth <= 0) { \
MUTEX_LOCK(&PL_lc_numeric_mutex); \
PL_lc_numeric_mutex_depth = 1; \
DEBUG_Lv(PerlIO_printf(Perl_debug_log, \
"%s: %d: locking lc_numeric; depth=1\n", \
__FILE__, __LINE__)); \
} \
else { \
PL_lc_numeric_mutex_depth++; \
DEBUG_Lv(PerlIO_printf(Perl_debug_log, \
"%s: %d: avoided lc_numeric_lock; new depth=%d\n", \
__FILE__, __LINE__, PL_lc_numeric_mutex_depth)); \
if (cond_to_panic_if_already_locked) { \
locale_panic_("Trying to change LC_NUMERIC incompatibly");\
} \
} \
} STMT_END

# define LC_NUMERIC_UNLOCK \
STMT_START { \
if (PL_lc_numeric_mutex_depth <= 1) { \
MUTEX_UNLOCK(&PL_lc_numeric_mutex); \
PL_lc_numeric_mutex_depth = 0; \
DEBUG_Lv(PerlIO_printf(Perl_debug_log, \
"%s: %d: unlocking lc_numeric; depth=0\n", \
__FILE__, __LINE__)); \
} \
else { \
PL_lc_numeric_mutex_depth--; \
DEBUG_Lv(PerlIO_printf(Perl_debug_log, \
"%s: %d: avoided lc_numeric_unlock; new depth=%d\n",\
__FILE__, __LINE__, PL_lc_numeric_mutex_depth)); \
} \
} STMT_END \
CLANG_DIAG_RESTORE

# define LOCALE_INIT_LC_NUMERIC_ MUTEX_INIT(&PL_lc_numeric_mutex)
# define LOCALE_TERM_LC_NUMERIC_ MUTEX_DESTROY(&PL_lc_numeric_mutex)
# endif
#endif

#ifndef LC_NUMERIC_LOCK
# define LC_NUMERIC_LOCK(cond) NOOP
# define LC_NUMERIC_UNLOCK NOOP
#endif

#ifdef USE_LOCALE_NUMERIC

/* These macros are for toggling between the underlying locale (UNDERLYING or
Expand Down
3 changes: 0 additions & 3 deletions perlvars.h
View file
Expand Up @@ -106,9 +106,6 @@ PERLVARI(G, mmap_page_size, IV, 0)
PERLVAR(G, hints_mutex, perl_mutex) /* Mutex for refcounted he refcounting */
PERLVAR(G, env_mutex, perl_RnW1_mutex_t) /* Mutex for accessing ENV */
PERLVAR(G, locale_mutex, perl_mutex) /* Mutex related to locale handling */
# ifndef USE_THREAD_SAFE_LOCALE
PERLVAR(G, lc_numeric_mutex, perl_mutex) /* Mutex for switching LC_NUMERIC */
# endif
#endif

#ifdef USE_POSIX_2008_LOCALE
Expand Down
3 changes: 0 additions & 3 deletions sv.c
View file
Expand Up @@ -15344,9 +15344,6 @@ perl_clone_using(PerlInterpreter *proto_perl, UV flags,
#if defined(USE_LOCALE) && defined(USE_LOCALE_THREADS)
assert(PL_locale_mutex_depth <= 0);
PL_locale_mutex_depth = 0;
#endif
#if defined(USE_ITHREADS) && ! defined(USE_THREAD_SAFE_LOCALE)
PL_lc_numeric_mutex_depth = 0;
#endif
/* Unicode features (see perlrun/-C) */
PL_unicode = proto_perl->Iunicode;
Expand Down

0 comments on commit 7996b1c

Please sign in to comment.