[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Locks and threads
From: |
Ludovic Courtès |
Subject: |
Re: Locks and threads |
Date: |
Thu, 26 Mar 2009 10:10:36 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.0.90 (gnu/linux) |
Hi Neil,
Neil Jerram <address@hidden> writes:
> - it runs for a duration specified by environment variable
> GUILE_TEST_DEFINE_RACE_DURATION
>
> - it defaults to just 10s if that variable isn't defined.
Even 10s is going to be slightly annoying. Another option would be to
create tens of guile-definer threads instead of just 4; it might trigger
the problem earlier.
> #2 makes the symbols hash thread-safe, and it appears that this
> completely fixes the define-race problem. I don't understand why we
> apparently don't need patch #3 as well - but that's what my results
> indicate.
Roughly #2 is a specialized version of #3 as it addresses only the case
of the symbol hash table. So it should be enough for this very problem.
#3 is probably the way to go in the longer run, but it changes the
API/ABI so it can't go in 1.8.
I'll skip #3 for now.
> * test-suite/standalone/test-define-race.c: New test.
Could you run `indent' on this file?
> +TESTS += test-define-race
It needs to be conditionalized on `--with-threads'.
> scm_i_rehash (SCM table,
> unsigned long (*hash_fn)(),
> void *closure,
> - const char* func_name)
> + const char* func_name,
> + scm_i_pthread_mutex_t *mutex)
This function assumes that MUTEX is locked. I would either write it
prominently in a comment above or change the semantics so that MUTEX is
acquired in `scm_i_rehash ()', not in the caller.
The latter might be achieved by integrating tests about whether TABLE
needs to be rehashed into `scm_i_rehash ()' itself. I.e., this:
if (SCM_HASHTABLE_N_ITEMS (table) < SCM_HASHTABLE_LOWER (table)
|| SCM_HASHTABLE_N_ITEMS (table) > SCM_HASHTABLE_UPPER (table))
scm_i_rehash (...);
would become:
scm_i_rehash (...);
I haven't reviewed all uses to see whether it's actually possible and
whether it would lead to race conditions, though.
> -static SCM symbols;
> +SCM scm_i_symbols;
I don't think this is needed, is it?
> +static SCM
> +intern_symbol (SCM symbol, const char *name, size_t len, size_t raw_hash)
Since all users of `intern_symbol ()' perform a
`lookup_interned_symbol ()' right before, I'd rather change users so
that they lock before `lookup' and unlock after `intern':
lock (&symbols_mutex);
sym = lookup_interned_symbol (name);
if (scm_is_false (sym))
{
sym = scm_i_c_make_symbol (...);
intern_symbol (sym);
}
unlock (&symbols_mutex);
Is it doable?
Thanks,
Ludo'.
- Re: Locks and threads, (continued)
- Re: Locks and threads, Andy Wingo, 2009/03/12
- Re: Locks and threads, Neil Jerram, 2009/03/13
- Re: Locks and threads, Neil Jerram, 2009/03/25
- Re: Locks and threads, Linas Vepstas, 2009/03/25
- Re: Locks and threads, Neil Jerram, 2009/03/26
- Re: Locks and threads, Linas Vepstas, 2009/03/26
- Re: Locks and threads,
Ludovic Courtès <=
- Re: Locks and threads, Neil Jerram, 2009/03/26
- Re: Locks and threads, Ludovic Courtès, 2009/03/26
- Re: Locks and threads, Neil Jerram, 2009/03/26
- Re: Locks and threads, Linas Vepstas, 2009/03/26
- Re: Locks and threads, Ludovic Courtès, 2009/03/14
- Re: Locks and threads, Andy Wingo, 2009/03/16
Re: Locks and threads, Neil Jerram, 2009/03/25
Re: Locks and threads, Neil Jerram, 2009/03/05