1
Fork 0
mirror of https://git.savannah.gnu.org/git/guile.git synced 2025-04-30 03:40:34 +02:00

Don't hold lock during scm_async_tick in readdir and ttyname

Only hold scm_i_misc_mutex while making the C calls.  This also avoids
the need for a dynamic-wind.  Add SCM_I_LOCKED_SYSCALL (similar to
SCM_SYSCALL) to handle the locking and EINTR loop.

libguile/filesys.c (scm_readdir): rely on SCM_I_LOCKED_SYSCALL to limit
locking.
libguile/filesys.c (scm_ttyname): rely on SCM_I_LOCKED_SYSCALL to limit
locking.
libguile/syscalls.h: add SCM_I_LOCKED_SYSCALL.
This commit is contained in:
Rob Browning 2025-01-22 12:55:06 -06:00
parent 4a6347c371
commit 63756efbc5
4 changed files with 44 additions and 45 deletions

2
NEWS
View file

@ -69,6 +69,8 @@ every line in a file.
** Immutable stringbufs are now 8-byte aligned on all systems ** Immutable stringbufs are now 8-byte aligned on all systems
Previously they could end up with an alignment that violated the type Previously they could end up with an alignment that violated the type
tag for their type (e.g. ending up tagged as immediates SCM_IMP()). tag for their type (e.g. ending up tagged as immediates SCM_IMP()).
** `readdir` and `ttyname` now release scm_i_misc_mutex during asyncs
This avoids potential deadlocks.
Changes in 3.0.10 (since 3.0.9) Changes in 3.0.10 (since 3.0.9)

View file

@ -2231,30 +2231,22 @@ SCM_DEFINE (scm_readdir, "readdir", 1, 0, 0,
"end of file object is returned.") "end of file object is returned.")
#define FUNC_NAME s_scm_readdir #define FUNC_NAME s_scm_readdir
{ {
SCM ret;
scm_i_pthread_mutex_t *mutex;
struct dirent_or_dirent64 *rdent;
SCM_VALIDATE_DIR (1, port); SCM_VALIDATE_DIR (1, port);
if (!SCM_DIR_OPEN_P (port)) if (!SCM_DIR_OPEN_P (port))
SCM_MISC_ERROR ("Directory ~S is not open.", scm_list_1 (port)); SCM_MISC_ERROR ("Directory ~S is not open.", scm_list_1 (port));
mutex = (scm_i_pthread_mutex_t *) SCM_SMOB_DATA_2 (port); scm_i_pthread_mutex_t *mutex = (scm_i_pthread_mutex_t *) SCM_SMOB_DATA_2 (port);
DIR *dir = (DIR *) SCM_SMOB_DATA_1 (port);
scm_dynwind_begin (0); char *name = 0;
scm_i_dynwind_pthread_mutex_lock (mutex); SCM_I_LOCKED_SYSCALL
(mutex,
errno = 0; struct dirent_or_dirent64 *rdent = readdir_or_readdir64 (dir);
SCM_SYSCALL (rdent = readdir_or_readdir64 ((DIR *) SCM_SMOB_DATA_1 (port))); if (rdent) name = strdup (rdent->d_name));
if (errno != 0) if (name)
SCM_SYSERROR; return scm_take_locale_string (name);
if (!errno)
ret = (rdent ? scm_from_locale_stringn (rdent->d_name, NAMLEN (rdent)) return SCM_EOF_VAL;
: SCM_EOF_VAL); SCM_SYSERROR;
scm_dynwind_end ();
return ret;
} }
#undef FUNC_NAME #undef FUNC_NAME

View file

@ -1048,8 +1048,10 @@ SCM_DEFINE (scm_getsid, "getsid", 1, 0, 0,
continuously calling ttyname will otherwise get an overwrite quite continuously calling ttyname will otherwise get an overwrite quite
easily. easily.
ttyname_r (when available) could be used instead of scm_i_misc_mutex, but ttyname_r (when available) could be used instead of scm_i_misc_mutex
there's probably little to be gained in either speed or parallelism. */ if it doesn't restrict the maximum name length the way readdir_r can,
but there's probably little to be gained in either speed or
parallelism. */
#ifdef HAVE_TTYNAME #ifdef HAVE_TTYNAME
SCM_DEFINE (scm_ttyname, "ttyname", 1, 0, 0, SCM_DEFINE (scm_ttyname, "ttyname", 1, 0, 0,
@ -1058,34 +1060,19 @@ SCM_DEFINE (scm_ttyname, "ttyname", 1, 0, 0,
"underlying @var{port}.") "underlying @var{port}.")
#define FUNC_NAME s_scm_ttyname #define FUNC_NAME s_scm_ttyname
{ {
char *result;
int fd, err;
SCM ret = SCM_BOOL_F;
port = SCM_COERCE_OUTPORT (port); port = SCM_COERCE_OUTPORT (port);
SCM_VALIDATE_OPPORT (1, port); SCM_VALIDATE_OPPORT (1, port);
if (!SCM_FPORTP (port)) if (!SCM_FPORTP (port))
return SCM_BOOL_F; return SCM_BOOL_F;
fd = SCM_FPORT_FDES (port);
scm_i_scm_pthread_mutex_lock (&scm_i_misc_mutex); int fd = SCM_FPORT_FDES (port);
char *name = 0;
SCM_SYSCALL (result = ttyname (fd)); SCM_I_LOCKED_SYSCALL(&scm_i_misc_mutex,
err = errno; char *n = ttyname (fd);
if (result != NULL) if (n) name = strdup (n));
result = strdup (result); if (name)
return scm_take_locale_string (name);
scm_i_pthread_mutex_unlock (&scm_i_misc_mutex); SCM_SYSERROR;
if (!result)
{
errno = err;
SCM_SYSERROR;
}
else
ret = scm_take_locale_string (result);
return ret;
} }
#undef FUNC_NAME #undef FUNC_NAME
#endif /* HAVE_TTYNAME */ #endif /* HAVE_TTYNAME */

View file

@ -37,6 +37,24 @@
} \ } \
while (errno == EINTR) while (errno == EINTR)
/* A SCM_SYSCALL for calls that need mutex serialization. The body must
ensure to leave an appropriate errno. */
#define SCM_I_LOCKED_SYSCALL(lock, body) \
while(1) \
{ \
scm_i_pthread_mutex_t *lock___ = (lock); \
scm_i_pthread_mutex_lock (lock___); \
errno = 0; \
{ body; } \
int err___ = errno; \
scm_i_pthread_mutex_unlock (lock___); \
if (err___ != EINTR) \
{ \
errno = err___; \
break; \
} \
scm_async_tick (); \
}