[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Does Guile have a thread limit?
From: |
Taylan Ulrich Bayırlı /Kammer |
Subject: |
Re: Does Guile have a thread limit? |
Date: |
Thu, 10 Apr 2014 23:22:14 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (berkeley-unix) |
address@hidden (Ludovic Courtès) writes:
> address@hidden (Taylan Ulrich "Bayırlı/Kammer") skribis:
>
>> OK, I expected this. We could just make that one abort then, and
>> keep the exception in `call-with-new-thread' and similar. (Or are
>> there other plausible ways to signal an error in C, other than return
>> value and error-pointer argument?)
>
> No, that’s a problem.
>
>> After that change is done, will we be able to guarantee that thread
>> initialization is *always* successful?
>
> No, we can never guarantee that.
>
> We could introduce a variant of scm_with_guile2, but I’m not sure
> there’s anything sensible that can be done anyway.
>
> What matters, though, is for Scheme-level procedures that create
> threads to not abort.
>
> Ludo’.
Here's an improved patch. Other from the minor issue of using the
phrase "thread initialization" more consistently (in place of "creation"
or "launching"), it has the following differences:
- scm_with_guile() doesn't take an `int *err' argument, it aborts on
error. While an abort is nasty, the only alternative I see is a
silent failure, which I think is worse; correct me if I'm wrong.
- There is scm_with_guile_safe() which takes an `int *err'.
- The `int err' fields of launch_data and spawn_data structs aren't
written to without locking the mutex (since they're being read from
the other thread); this might have been the reason of some segfaults I
got (though I can't really imagine how, because it should only happen
when thread initialization *does* fail, which isn't triggered by
anything in the test-suite).
While C code using up many FDs and then calling scm_with_guile() might
abort --which should be a rare case--, `call-with-new-thread' neatly
raises an exception ... almost. In the REPL, when I repeatedly enter
our test-case snippet of launching 1k sleeping threads, I manage to make
Guile hang. In case anyone's interested, here's the backtraces of all
threads when I attach with GDB: http://sprunge.us/VZMY
>From 37cbfdedfc0523f52f7d45f737998a4a45ff9ffa Mon Sep 17 00:00:00 2001
From: Taylan Ulrich B <address@hidden>
Date: Thu, 10 Apr 2014 17:29:28 +0200
Subject: [PATCH] Handle thread initialization failure
* libguile/init.h (scm_init_guile): Return int for errors.
* libguile/threads.h (scm_with_guile_safe): New function.
(scm_threads_prehistory): Return int for error.
(scm_init_threads): Return int for errors.
* libguile/threads.c (guilify_self_1): Return int for errors.
(guilify_self_2): Return int for errors.
(scm_i_init_thread_for_guile): Return -1 on failure.
(scm_init_guile): Return int for errors.
(with_guile_args): Add `int err' field.
(with_guile_and_parent): Handle thread initialization failure.
(scm_i_with_guile_and_parent): Handle `with_guile_and_parent' error.
(scm_with_guile): Abort on failure (as before, just explicitly).
(scm_with_guile_safe): Non-aborting variant, takes `int *err'.
(launch_data): Add `int err' field.
(launch_thread): Handle `scm_i_with_guile_and_parent' error.
(call-with-new-thread): Signal thread initialization failure.
(spawn_data): Add `int err' field.
(spawn_thread): Signal thread initialization failure.
(scm_threads_prehistory): Return int for errors.
(scm_init_threads): Return int for errors.
---
libguile/init.h | 2 +-
libguile/threads.c | 108 +++++++++++++++++++++++++++++++++++++++++------------
libguile/threads.h | 5 ++-
3 files changed, 88 insertions(+), 27 deletions(-)
diff --git a/libguile/init.h b/libguile/init.h
index bc6cddf..f529c4f 100644
--- a/libguile/init.h
+++ b/libguile/init.h
@@ -30,7 +30,7 @@
SCM_INTERNAL scm_i_pthread_mutex_t scm_i_init_mutex;
SCM_API int scm_initialized_p;
-SCM_API void scm_init_guile (void);
+SCM_API int scm_init_guile (void);
SCM_API void scm_boot_guile (int argc, char **argv,
void (*main_func) (void *closure,
diff --git a/libguile/threads.c b/libguile/threads.c
index dd04f6f..754d517 100644
--- a/libguile/threads.c
+++ b/libguile/threads.c
@@ -394,7 +394,7 @@ scm_i_reset_fluid (size_t n)
/* Perform first stage of thread initialisation, in non-guile mode.
*/
-static void
+static int
guilify_self_1 (struct GC_stack_base *base)
{
scm_i_thread t;
@@ -434,10 +434,7 @@ guilify_self_1 (struct GC_stack_base *base)
t.vp = NULL;
if (pipe2 (t.sleep_pipe, O_CLOEXEC) != 0)
- /* FIXME: Error conditions during the initialization phase are handled
- gracelessly since public functions such as `scm_init_guile ()'
- currently have type `void'. */
- abort ();
+ return -1;
scm_i_pthread_mutex_init (&t.admin_mutex, NULL);
t.canceled = 0;
@@ -467,11 +464,13 @@ guilify_self_1 (struct GC_stack_base *base)
GC_enable ();
}
+
+ return 0;
}
/* Perform second stage of thread initialisation, in guile mode.
*/
-static void
+static int
guilify_self_2 (SCM parent)
{
scm_i_thread *t = SCM_I_CURRENT_THREAD;
@@ -503,6 +502,8 @@ guilify_self_2 (SCM parent)
/* See note in finalizers.c:queue_finalizer_async(). */
GC_invoke_finalizers ();
+
+ return 0;
}
@@ -683,7 +684,7 @@ init_thread_key (void)
which case the default dynamic state is used.
Returns zero when the thread was known to guile already; otherwise
- return 1.
+ return 1 for success, -1 for failure.
Note that it could be the case that the thread was known
to Guile, but not in guile mode (because we are within a
@@ -733,21 +734,23 @@ scm_i_init_thread_for_guile (struct GC_stack_base *base,
SCM parent)
GC_register_my_thread (base);
#endif
- guilify_self_1 (base);
- guilify_self_2 (parent);
+ if (guilify_self_1 (base) != 0)
+ return -1;
+ if (guilify_self_2 (parent) != 0)
+ return -1;
}
return 1;
}
}
-void
+int
scm_init_guile ()
{
struct GC_stack_base stack_base;
if (GC_get_stack_base (&stack_base) == GC_SUCCESS)
- scm_i_init_thread_for_guile (&stack_base,
- scm_i_default_dynamic_state);
+ return scm_i_init_thread_for_guile (&stack_base,
+ scm_i_default_dynamic_state);
else
{
fprintf (stderr, "Failed to get stack base for current thread.\n");
@@ -760,6 +763,7 @@ struct with_guile_args
GC_fn_type func;
void *data;
SCM parent;
+ int err;
};
static void *
@@ -779,6 +783,11 @@ with_guile_and_parent (struct GC_stack_base *base, void
*data)
struct with_guile_args *args = data;
new_thread = scm_i_init_thread_for_guile (base, args->parent);
+ if (new_thread < 0)
+ {
+ args->err = new_thread;
+ return NULL;
+ }
t = SCM_I_CURRENT_THREAD;
if (new_thread)
{
@@ -820,22 +829,44 @@ with_guile_and_parent (struct GC_stack_base *base, void
*data)
}
static void *
-scm_i_with_guile_and_parent (void *(*func)(void *), void *data, SCM parent)
+scm_i_with_guile_and_parent (void *(*func)(void *), void *data, SCM parent,
+ int *err)
{
struct with_guile_args args;
+ void *result;
args.func = func;
args.data = data;
args.parent = parent;
-
- return GC_call_with_stack_base (with_guile_and_parent, &args);
+ args.err = 0;
+
+ result = GC_call_with_stack_base (with_guile_and_parent, &args);
+ if (err)
+ *err = args.err;
+ return result;
}
void *
scm_with_guile (void *(*func)(void *), void *data)
{
+ void *result;
+ int err = 0;
+
+ result = scm_i_with_guile_and_parent (func, data,
+ scm_i_default_dynamic_state,
+ &err);
+ if (err)
+ abort ();
+
+ return result;
+}
+
+void *
+scm_with_guile_safe (void *(*func)(void *), void *data, int *err)
+{
return scm_i_with_guile_and_parent (func, data,
- scm_i_default_dynamic_state);
+ scm_i_default_dynamic_state,
+ err);
}
void *
@@ -867,6 +898,7 @@ typedef struct {
SCM thread;
scm_i_pthread_mutex_t mutex;
scm_i_pthread_cond_t cond;
+ int err;
} launch_data;
static void *
@@ -895,8 +927,16 @@ static void *
launch_thread (void *d)
{
launch_data *data = (launch_data *)d;
+ int err = 0;
scm_i_pthread_detach (scm_i_pthread_self ());
- scm_i_with_guile_and_parent (really_launch, d, data->parent);
+ scm_i_with_guile_and_parent (really_launch, d, data->parent, &err);
+ if (err)
+ {
+ scm_i_pthread_mutex_lock (&data->mutex);
+ data->err = err;
+ scm_i_pthread_cond_signal (&data->cond);
+ scm_i_pthread_mutex_unlock (&data->mutex);
+ }
return NULL;
}
@@ -929,6 +969,7 @@ SCM_DEFINE (scm_call_with_new_thread,
"call-with-new-thread", 1, 1, 0,
data.thread = SCM_BOOL_F;
scm_i_pthread_mutex_init (&data.mutex, NULL);
scm_i_pthread_cond_init (&data.cond, NULL);
+ data.err = 0;
scm_i_scm_pthread_mutex_lock (&data.mutex);
err = scm_i_pthread_create (&id, NULL, launch_thread, &data);
@@ -939,11 +980,14 @@ SCM_DEFINE (scm_call_with_new_thread,
"call-with-new-thread", 1, 1, 0,
scm_syserror (NULL);
}
- while (scm_is_false (data.thread))
+ while (scm_is_false (data.thread) && data.err == 0)
scm_i_scm_pthread_cond_wait (&data.cond, &data.mutex);
scm_i_pthread_mutex_unlock (&data.mutex);
+ if (data.err)
+ SCM_MISC_ERROR ("could not initialize thread", SCM_EOL);
+
return data.thread;
}
#undef FUNC_NAME
@@ -957,6 +1001,7 @@ typedef struct {
SCM thread;
scm_i_pthread_mutex_t mutex;
scm_i_pthread_cond_t cond;
+ int err;
} spawn_data;
static void *
@@ -988,8 +1033,16 @@ static void *
spawn_thread (void *d)
{
spawn_data *data = (spawn_data *)d;
+ int err = 0;
scm_i_pthread_detach (scm_i_pthread_self ());
- scm_i_with_guile_and_parent (really_spawn, d, data->parent);
+ scm_i_with_guile_and_parent (really_spawn, d, data->parent, &err);
+ if (err)
+ {
+ scm_i_pthread_mutex_lock (&data->mutex);
+ data->err = err;
+ scm_i_pthread_cond_signal (&data->cond);
+ scm_i_pthread_mutex_unlock (&data->mutex);
+ }
return NULL;
}
@@ -1009,6 +1062,7 @@ scm_spawn_thread (scm_t_catch_body body, void *body_data,
data.thread = SCM_BOOL_F;
scm_i_pthread_mutex_init (&data.mutex, NULL);
scm_i_pthread_cond_init (&data.cond, NULL);
+ data.err = 0;
scm_i_scm_pthread_mutex_lock (&data.mutex);
err = scm_i_pthread_create (&id, NULL, spawn_thread, &data);
@@ -1019,11 +1073,14 @@ scm_spawn_thread (scm_t_catch_body body, void
*body_data,
scm_syserror (NULL);
}
- while (scm_is_false (data.thread))
+ while (scm_is_false (data.thread) && data.err == 0)
scm_i_scm_pthread_cond_wait (&data.cond, &data.mutex);
scm_i_pthread_mutex_unlock (&data.mutex);
+ if (data.err)
+ scm_misc_error (NULL, "could not spawn thread", SCM_EOL);
+
assert (SCM_I_IS_THREAD (data.thread));
return data.thread;
@@ -2070,7 +2127,7 @@ scm_i_pthread_mutex_t scm_i_misc_mutex;
pthread_mutexattr_t scm_i_pthread_mutexattr_recursive[1];
#endif
-void
+int
scm_threads_prehistory (void *base)
{
#if SCM_USE_PTHREAD_THREADS
@@ -2089,14 +2146,14 @@ scm_threads_prehistory (void *base)
GC_MAKE_PROC (GC_new_proc (thread_mark), 0),
0, 1);
- guilify_self_1 ((struct GC_stack_base *) base);
+ return guilify_self_1 ((struct GC_stack_base *) base);
}
scm_t_bits scm_tc16_thread;
scm_t_bits scm_tc16_mutex;
scm_t_bits scm_tc16_condvar;
-void
+int
scm_init_threads ()
{
scm_tc16_thread = scm_make_smob_type ("thread", sizeof (scm_i_thread));
@@ -2110,10 +2167,13 @@ scm_init_threads ()
scm_set_smob_print (scm_tc16_condvar, fat_cond_print);
scm_i_default_dynamic_state = SCM_BOOL_F;
- guilify_self_2 (SCM_BOOL_F);
+ if (guilify_self_2 (SCM_BOOL_F) != 0)
+ return -1;
threads_initialized_p = 1;
dynwind_critical_section_mutex = scm_make_recursive_mutex ();
+
+ return 0;
}
void
diff --git a/libguile/threads.h b/libguile/threads.h
index 6b85baf..60d6a64 100644
--- a/libguile/threads.h
+++ b/libguile/threads.h
@@ -137,10 +137,11 @@ SCM_API SCM scm_spawn_thread (scm_t_catch_body body, void
*body_data,
SCM_API void *scm_without_guile (void *(*func)(void *), void *data);
SCM_API void *scm_with_guile (void *(*func)(void *), void *data);
+SCM_API void *scm_with_guile_safe (void *(*func)(void *), void *data, int
*err);
SCM_INTERNAL void scm_i_reset_fluid (size_t);
-SCM_INTERNAL void scm_threads_prehistory (void *);
-SCM_INTERNAL void scm_init_threads (void);
+SCM_INTERNAL int scm_threads_prehistory (void *);
+SCM_INTERNAL int scm_init_threads (void);
SCM_INTERNAL void scm_init_thread_procs (void);
SCM_INTERNAL void scm_init_threads_default_dynamic_state (void);
--
1.8.1.2
- Does Guile have a thread limit?, Diogo F. S. Ramos, 2014/04/05
- Re: Does Guile have a thread limit?, Eli Zaretskii, 2014/04/05
- Re: Does Guile have a thread limit?, Mark H Weaver, 2014/04/06
- Re: Does Guile have a thread limit?, Taylan Ulrich Bayırlı /Kammer, 2014/04/08
- Re: Does Guile have a thread limit?, Ludovic Courtès, 2014/04/08
- Re: Does Guile have a thread limit?, Taylan Ulrich Bayırlı /Kammer, 2014/04/08
- Re: Does Guile have a thread limit?, Ludovic Courtès, 2014/04/10
- Re: Does Guile have a thread limit?,
Taylan Ulrich Bayırlı /Kammer <=
- Re: Does Guile have a thread limit?, Taylan Ulrich Bayirli/Kammer, 2014/04/25
- Re: Does Guile have a thread limit?, Mark H Weaver, 2014/04/26
- Re: Does Guile have a thread limit?, Taylan Ulrich Bayirli/Kammer, 2014/04/26