bug#32786: (system* ...) call somehow interferes with atomic-box on armv7l

[Mark H Weaver <mhw@netris.org>]

Hi,

Михаил Бахтерев <mob@k.imm.uran.ru> writes:

> I attach the code sample, which expected behaviour is:
>
>   1. to periodically restart thread, which executes «sleep 1s» with
>   system* call;
>
>   2. and then to check if it should repeat the execution.
>   
> The flag, which controls that re-execution is managed by another thread
> through atomic-box primitive.
>
> The code does not run as expected: the sleep executing sleep starts
> once, and change seems to remain invisible to main thread.

Can you describe in more detail the behavior you are seeing?  What
messages do you see on stderr from your 'dump' calls when it gets stuck?
What state is the atomic box left in?

I see a couple of problems with the code below:

* Two threads write concurrently to the same port, namely
  (current-error-port).  You should use thread synchronization to ensure
  that operations to a given port are serialized.

* There's a race condition when the atomic box is in the #:accepted
  state.  If the main thread finds the box in that state, it changes the
  state to #:need-to-sleep, which will apparently cause the other thread
  to sleep again.

I'm not sure if these problems could explain what you're seeing.

     Regards,
       Mark


> The code works as expected, when:
>
>   1. (sleep 1) is used instead (system* "sleep" "1s");
>   2. running the code on x86_64 cpus.
>
> The use of affinity mask
>
>   taskset 0x01 guile test.scm
>
> does not help.
>
> - MB. Respectfully
>
> P.S. Additional information:
>
> $ guile --version  
> guile (GNU Guile) 2.2.4
> Copyright (C) 2018 Free Software Foundation, Inc.
>
> License LGPLv3+: GNU LGPL 3 or later <http://gnu.org/licenses/lgpl.html>.
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.
>
> $ sh config.guess
> armv7l-unknown-linux-gnueabihf
>
> $ pacman -Qi guile 
> Name            : guile
> Version         : 2.2.4-1
> Description     : Portable, embeddable Scheme implementation written in C
> Architecture    : armv7h
> URL             : https://www.gnu.org/software/guile/
> Licenses        : GPL
> Groups          : None
> Provides        : None
> Depends On      : gmp  libltdl  ncurses  texinfo  libunistring  gc  libffi
> Optional Deps   : None
> Required By     : make
> Optional For    : gnutls
> Conflicts With  : None
> Replaces        : None
> Installed Size  : 40.83 MiB
> Packager        : Arch Linux ARM Build System <builder+xu9@archlinuxarm.org>
> Build Date      : Tue 03 Jul 2018 04:47:53 PM +05
> Install Date    : Mon 09 Jul 2018 01:02:48 PM +05
> Install Reason  : Installed as a dependency for another package
> Install Script  : No
> Validated By    : Signature
>
> (use-modules (ice-9 atomic)
>              (ice-9 threads))
>
> (define dump
>   (let ((p (current-error-port)))
>     (lambda (fmt . args) (apply format p fmt args))))
>
> (define state (make-atomic-box #:nothing-to-do))
>
> (define (expect e v)
>   (when (not (eq? e v))
>     (error "Protocol violation; (expecting . got):" (cons e v)))) 
>
> (define (sleep-loop)
>   (expect #:need-to-sleep (atomic-box-ref state))
>
>   (dump "R: Going to (sleep 1)~%")
>   (atomic-box-set! state #:accepted)
>   
>   ; SOMETHING WRONG IS HERE
>   (system* "sleep" "1s")
>   ; (sleep 1)
>
>   (let ((v (atomic-box-compare-and-swap! state #:accepted #:nothing-to-do)))
>     (when (not (eq? #:accepted v))
>       (dump "S: Repeating sleep~%")
>       (sleep-loop)))
>
>   (dump "R: sleep-loop finished~%"))
>
> (define (main-loop)
>   (define (run-thread)
>     (dump "M: new sleep thread~%")
>     (atomic-box-set! state #:need-to-sleep)
>     (call-with-new-thread sleep-loop)) 
>
>   (dump "M: (sleep 3)~%")
>   (sleep 3)
>
>   (dump "M: protocol exchange~%")
>   (let ((st (atomic-box-ref state)))
>     (case st 
>       ((#:nothing-to-do) (run-thread))
>       ((#:accepted) (let ((v (atomic-box-compare-and-swap! state #:accepted #:need-to-sleep)))
>                       (when (not (eq? #:accepted v))
>                         (expect #:accepted v)
>                         (run-thread))))
>       (else (expect #:need-to-sleep st))))
>
>   (main-loop))
>
> (main-loop)