Re: Race condition in threading code?

[Han-Wen Nienhuys <hanwen@xs4all.nl>]

Ludovic Courtès escreveu:
> Hello,
> 
> Andy Wingo <wingo@pobox.com> writes:
> 
>> ERROR: srfi-18.test: thread-start!:
>>   thread activates only after start
>>    - arguments: ((syntax-error "memoization"
>>                   "In file ~S, line ~S: ~A ~S in expression ~S."
>>                   ("/home/lilydev/vc/guile/srfi/srfi-18.scm" 135
>>                    "Bad binding" ct
>>                     (let (ct (current-thread))
>>                     #@let (or (hashq-ref thread-exception-handlers ct)
>>                           (hashq-set! thread-exception-handlers ct
>>                     (list initial-handler))))) #f))
> 
> I'm seeing this as well, but it's a `#@let*' here (single-binding `let's
> are memoized as `#@let*'):
> 
>   ((syntax-error "memoization"
>                  "In file ~S, line ~S: ~A ~S in expression ~S."
>                  ("/home/ludo/src/guile/srfi/srfi-18.scm" 138
>                   "Bad binding"
>                   ct
>                   (#@let* (ct (#<variable b7d28110 value: #<primitive-procedure current-thread>>))
>                     (#@or (#<variable b7d2ad88 value: #<primitive-procedure hashq-ref>>
>                                       #<variable 839df08 value: #<weak-key-hash-table 1/31>> #@0-0)
>                           (#<variable b7d2adc0 value: #<primitive-procedure hashq-set!>> #<variable 839df08 value: #<weak-key-hash-table 1/31>> #@0-0 (#<variable b7d2c498 value: #<primitive-procedure list>> #<variable 839d130 value: #<procedure initial-handler (obj)>>))
>                           )))
>                  #f))
> 
> It can be reproduced, but very infrequently, with this program:
> 
>   (use-modules (ice-9 threads))
> 
>   (define (foo x y)
>     (let ((z (+ x y)))
>       (let ((a (+ z 1)))
>         (let ((b (- a 2)))
>           (let ((c (* b 3)))
>             c)))))
> 
>   (define (entry)
>     (foo 1 2))
> 
>   (for-each (lambda (i) (make-thread entry))
>             (iota 123))
> 
> My explanation is that the `let*' memoizer, aka. `scm_m_letstar ()', is
> not thread-safe; it's clearly not atomic, and it's of course not
> protected by a mutex or so.

Is that the only one?

  SCM
  scm_m_let (SCM expr, SCM env)
  ...
      /* plain let */
      SCM rvariables;
      SCM inits;
      transform_bindings (bindings, expr, &rvariables, &inits);

      {
        const SCM new_body = m_body (SCM_IM_LET, SCM_CDR (cdr_expr));
        const SCM new_tail = scm_cons2 (rvariables, inits, new_body);
        SCM_SETCAR (expr, SCM_IM_LET); 
        // ****!!!
        SCM_SETCDR (expr, new_tail);

What happens if another thread tries to evaluate expr at the place marked ****!!! ?

At the very least, we should have an atomic SCM_SETCELL() which overwrites car and
cdr atomically.

-- 
 Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen