fibers does not support wait-operation being interrupted, even when on a non-fibers thread #99

abcdw commented 2023年10月06日 11:15:20 +02:00 (Migrated from github.com)

I was experimenting with system-async-mark and found the problem with condition variables. I guess it happens because when async does non-local exit, the thread can not be removed from condition variable waiters and signaling fiber is hanging indefinitely.

The following code hangs on signal-condition! invocation:

(begin
 (format #t "\n\n====================\n")
 (use-modules (fibers) (fibers conditions) (ice-9 threads)
 (fibers channels))
 (define cnd (make-condition))
 (define ch (make-channel))
 (define mutex (make-mutex 'allow-external-unlock))
 (lock-mutex mutex)
 (define prompt (make-prompt-tag "tmp"))
 (define th (call-with-new-thread
 (lambda ()
 (call-with-prompt prompt
 (lambda ()
 (format #t "-> inside prompt\n")
 ;; (lock-mutex mutex)
 (wait cnd)
 (format #t "-> condition unlcoked\n"))
 (lambda (k . args)
 (apply values args)))
 (format #t "<- after prompt\n")
 ;; (wait cnd)
 ;; (lock-mutex mutex)
 (format #t "<- finishing\n"))))
 (sleep 1)
 (format #t "substituting computation\n")
 (system-async-mark
 (lambda ()
 (put-message ch 'hello)
 (abort-to-prompt prompt)) th)
 (run-fibers (lambda ()
 (format #t "message: ~a\n" (get-message ch))
 ;; (unlock-mutex mutex)
 (signal-condition! cnd)
 (format #t "!condition signalled\n"))
 #:drain? #f)
 (sleep 1)
 (format #t "after lock\n")
 (format #t "--------------------\n")
 (sleep 3) "")

The output is following:

====================
-> inside prompt
substituting computation
message: hello
<- after prompt
<- finishing

I was experimenting with system-async-mark and found the problem with condition variables. I guess it happens because when async does non-local exit, the thread can not be removed from condition variable waiters and signaling fiber is hanging indefinitely. The following code hangs on `signal-condition!` invocation: ``` (begin (format #t "\n\n====================\n") (use-modules (fibers) (fibers conditions) (ice-9 threads) (fibers channels)) (define cnd (make-condition)) (define ch (make-channel)) (define mutex (make-mutex 'allow-external-unlock)) (lock-mutex mutex) (define prompt (make-prompt-tag "tmp")) (define th (call-with-new-thread (lambda () (call-with-prompt prompt (lambda () (format #t "-> inside prompt\n") ;; (lock-mutex mutex) (wait cnd) (format #t "-> condition unlcoked\n")) (lambda (k . args) (apply values args))) (format #t "<- after prompt\n") ;; (wait cnd) ;; (lock-mutex mutex) (format #t "<- finishing\n")))) (sleep 1) (format #t "substituting computation\n") (system-async-mark (lambda () (put-message ch 'hello) (abort-to-prompt prompt)) th) (run-fibers (lambda () (format #t "message: ~a\n" (get-message ch)) ;; (unlock-mutex mutex) (signal-condition! cnd) (format #t "!condition signalled\n")) #:drain? #f) (sleep 1) (format #t "after lock\n") (format #t "--------------------\n") (sleep 3) "") ``` The output is following: ``` ==================== -> inside prompt substituting computation message: hello <- after prompt <- finishing ```

emixa-d commented 2023年12月18日 18:17:43 +01:00 (Migrated from github.com)

That code is broken in the first place -- the async from system-async-mark can be run inside another fiber (or even not in a fiber at all, e.g. Fiber's scheduling code), in particular it could be somewhere outside the call-with-prompt.

So, what's this code supposed to implement in the first place?

That code is broken in the first place -- the async from system-async-mark can be run inside _another_ fiber (or even not in a fiber at all, e.g. Fiber's scheduling code), in particular it could be somewhere outside the call-with-prompt. So, what's this code supposed to implement in the first place?

abcdw commented 2023年12月25日 11:41:07 +01:00 (Migrated from github.com)

The lambda from system-async-mark will be run in a separate thread, not in a fiber.

The original goal is to implement interruptable evaluation and here is a working implementation:
git.sr.ht/~abcdw/guile-ares-rs@ee80744483/item/src/nrepl/server/evaluation.scm (L158)

The lambda from system-async-mark will be run in a separate thread, not in a fiber. The original goal is to implement interruptable evaluation and here is a working implementation: https://git.sr.ht/~abcdw/guile-ares-rs/tree/ee807444833a19bf3d7d281ab1afa5225147b72c/item/src/nrepl/server/evaluation.scm#L158

emixa-d commented 2023年12月26日 00:29:21 +01:00 (Migrated from github.com)

(system-async-mark
(lambda ()
(put-message ch 'hello)
(abort-to-prompt prompt)) th)

I did not notice the th here ...

That seems much more reasonable. I am, however, quoting what I wrote in https://github.com/wingo/fibers/issues/29#issuecomment-1858497720 (not all of it applies here, but enough does):

Also, performing Fibers operations inside an async is super skeevy. The Fibers stuff might be in an intermediate state, which usually is invisible to Fibers users but potentially not from asyncs, so now it might be messing things up. Or the async might be run inside a fiber that currently is in the progress of doing a put-message of its own, which put-message was not designed to handle, so that might mess things up. Or perhaps it is even run inside the process monitor, while it is doing (get-message (current-process-monitor)) -- I don't know what would happen then, but I doubt it is anything good.

Really, unless it's a pure operation or something ultra-basic like vector-set! or something that's actually documented to be allowed/supported, you shouldn't be assuming you can simply do things from an async.

Please remove this process-monitor stuff or actually implement & document the prerequisites in Fibers first.

(This applies more generally, beyond Scheme and Fibers, to any APIs with concurrency.)

> (system-async-mark > (lambda () > (put-message ch 'hello) > (abort-to-prompt prompt)) th) I did not notice the `th` here ... That seems much more reasonable. I am, however, quoting what I wrote in <https://github.com/wingo/fibers/issues/29#issuecomment-1858497720> (not all of it applies here, but enough does): > Also, performing Fibers operations inside an async is super skeevy. The Fibers stuff might be in an intermediate state, which usually is invisible to Fibers users but potentially not from asyncs, so now it might be messing things up. Or the async might be run inside a fiber that currently is in the progress of doing a put-message of its own, which put-message was not designed to handle, so that might mess things up. Or perhaps it is even run inside the process monitor, while it is doing (get-message (current-process-monitor)) -- I don't know what would happen then, but I doubt it is anything good. > > Really, unless it's a pure operation or something ultra-basic like vector-set! or something that's actually documented to be allowed/supported, you shouldn't be assuming you can simply do things from an async. > > Please remove this process-monitor stuff or actually implement & document the prerequisites in Fibers first. (This applies more generally, beyond Scheme and Fibers, to any APIs with concurrency.)

emixa-d commented 2023年12月26日 00:37:44 +01:00 (Migrated from github.com)

(Also, pipe trick would be convenient here.)

(Also, pipe trick would be convenient here.)

abcdw commented 2023年12月27日 07:42:15 +01:00 (Migrated from github.com)

@emixa-d What do you mean by pipe trick?

@emixa-d What do you mean by pipe trick?

emixa-d commented 2023年12月29日 15:15:48 +01:00 (Migrated from github.com)

See, e.g., https://web.stanford.edu/class/cs110l/slides/lecture-11.pdf:

Avoiding signal handling
●くろまる Anything substantial should not be done in a signal handler
●くろまる How can we handle signals, then?
●くろまる The "self-pipe" trick was invented in the early 90s:
●くろまる Create a pipe
●くろまる When you’re awaiting a signal, read from the pipe (this will block until
something is written to it)
●くろまる In the signal handler, write a single byte to the pipe

Pipes have a positive, but finite, buffer size, so make sure not to write too much to the pipe.
For waiting until the pipe is readable, there is wait-until-port-readable-operation, but beware of spurious wakeups, so when reading from the pipe you may need to parameterize current-read-waiter appropriately.

See, e.g., <https://web.stanford.edu/class/cs110l/slides/lecture-11.pdf>: > Avoiding signal handling > ●くろまる Anything substantial should not be done in a signal handler > ●くろまる How can we handle signals, then? > ●くろまる The "self-pipe" trick was invented in the early 90s: > ●くろまる Create a pipe > ●くろまる When you’re awaiting a signal, read from the pipe (this will block until something is written to it) > ●くろまる In the signal handler, write a single byte to the pipe Pipes have a positive, but finite, buffer size, so make sure not to write too much to the pipe. For waiting until the pipe is readable, there is `wait-until-port-readable-operation`, but beware of spurious wakeups, so when reading from the pipe you may need to parameterize `current-read-waiter` appropriately.