(Probably) current-read-writer / current-write-waiter becomes invalid in new (non-fibers) threads spawned from within a fiber #105

abcdw commented 2024年04月26日 10:02:40 +02:00 (Migrated from github.com)

I create a non-blocking socket inside the thread spawned inside the fiber with the following code:

(define-module (nonblocking-socket)
 #:use-module (ice-9 threads)
 #:use-module (fibers)
 #:export (create-socket))
(define* (create-socket
 #:key (port 1134))
 (define (make-default-socket family addr port)
 (let ((sock (socket PF_INET SOCK_STREAM 0)))
 ;; (fcntl sock F_SETFD FD_CLOEXEC)
 (fcntl sock F_SETFL (logior O_NONBLOCK (fcntl sock F_GETFL)))
 sock))
 (define tmp-socket
 (make-default-socket AF_INET INADDR_LOOPBACK 1234))
 (connect tmp-socket (make-socket-address AF_INET INADDR_LOOPBACK port))
 (close-port tmp-socket))
(define-public (tmp)
 (run-fibers
 (lambda ()
 (call-with-new-thread
 (lambda () (create-socket)))
 (sleep 1)
 'yay)))

and I receive

In nonblocking-socket.scm:
 13:2 3 (_)
In ice-9/suspendable-ports.scm:
 68:33 2 (_ #<input-output: socket 63> _ . _)
In fibers/scheduler.scm:
 355:13 1 WARNING: (nonblocking-socket): imported module (fibers) overrides core binding `sleep'
(suspend-current-task #<procedure 7f71a8f45c40 at fiber...>)
In ice-9/boot-9.scm:
 1676:22 0 (raise-exception _ #:continuable? _)
ice-9/boot-9.scm:1676:22: In procedure raise-exception:
In procedure struct-vtable: Wrong type argument in position 1 (expecting struct): #f

Expected behavior is to get Connection refused or succesfully connect if another socket on target port is listening.

I create a non-blocking socket inside the thread spawned inside the fiber with the following code: ```scheme (define-module (nonblocking-socket) #:use-module (ice-9 threads) #:use-module (fibers) #:export (create-socket)) (define* (create-socket #:key (port 1134)) (define (make-default-socket family addr port) (let ((sock (socket PF_INET SOCK_STREAM 0))) ;; (fcntl sock F_SETFD FD_CLOEXEC) (fcntl sock F_SETFL (logior O_NONBLOCK (fcntl sock F_GETFL))) sock)) (define tmp-socket (make-default-socket AF_INET INADDR_LOOPBACK 1234)) (connect tmp-socket (make-socket-address AF_INET INADDR_LOOPBACK port)) (close-port tmp-socket)) (define-public (tmp) (run-fibers (lambda () (call-with-new-thread (lambda () (create-socket))) (sleep 1) 'yay))) ``` and I receive ``` In nonblocking-socket.scm: 13:2 3 (_) In ice-9/suspendable-ports.scm: 68:33 2 (_ #<input-output: socket 63> _ . _) In fibers/scheduler.scm: 355:13 1 WARNING: (nonblocking-socket): imported module (fibers) overrides core binding `sleep' (suspend-current-task #<procedure 7f71a8f45c40 at fiber...>) In ice-9/boot-9.scm: 1676:22 0 (raise-exception _ #:continuable? _) ice-9/boot-9.scm:1676:22: In procedure raise-exception: In procedure struct-vtable: Wrong type argument in position 1 (expecting struct): #f ``` Expected behavior is to get `Connection refused` or succesfully connect if another socket on target port is listening.

abcdw commented 2024年04月26日 10:06:12 +02:00 (Migrated from github.com)

It affects guile-ares-rs, when using with Guix or other code, dealing with non-blocking sockets, because evaluation thread is get spawned from fibers handling incomming nREPL requests:
https://todo.sr.ht/~abcdw/tickets/7

It affects [guile-ares-rs](https://git.sr.ht/~abcdw/guile-ares-rs/), when using with Guix or other code, dealing with non-blocking sockets, because evaluation thread is get spawned from fibers handling incomming nREPL requests: https://todo.sr.ht/~abcdw/tickets/7

abcdw commented 2024年05月29日 09:52:02 +02:00 (Migrated from github.com)

It is somehow related to dynamic-state, if I try to create a socket in dynamic state captured before fibers setup, it works well:

(define-public (create-socket-in-thread-in-fiber)
 (let ((ds (current-dynamic-state)))
 (run-fibers
 (lambda ()
 (define th
 (call-with-new-thread
 (lambda ()
 (with-dynamic-state
 ds
 create-socket))))
 (join-thread th)))))

Or I can create a whole thread in dynamic extend captured outside of fibers, it will also work.

It is somehow related to dynamic-state, if I try to create a socket in dynamic state captured before fibers setup, it works well: ```scheme (define-public (create-socket-in-thread-in-fiber) (let ((ds (current-dynamic-state))) (run-fibers (lambda () (define th (call-with-new-thread (lambda () (with-dynamic-state ds create-socket)))) (join-thread th))))) ``` Or I can create a whole thread in dynamic extend captured outside of fibers, it will also work.

emixa-d commented 2024年10月30日 20:17:05 +01:00 (Migrated from github.com)

In ``fibers.scm` there is a comment

;; When a scheduler is passed explicitly, it could be there is no
;; current fiber; in that case the dynamic state probably doesn't
;; have the right right current-read-waiter /
;; current-write-waiter, so wrap the thunk.

Could you test setting current-read-waiter and current-write-waiter to (its value outside the run-fibers) around the call-with-new-thread (or around the create-socket), to test the hypothesis if this kind of thing is going on?

Assuming this is correct, first suspend-current-task should be modified to check if a current scheduler is defined before using it (and if not, do something like (error "tried to suspend current task, but no current scheduler is defined"), to make such situations clearer for the future. This shouldn't incur performance problems since a type check exists anyway.

Likewise, current-read-waiter could be modified to make an error message like "Fiber's read waiter can only be used in a fiber context, consider setting current-read-waiter appropriately."

Some text could be added to the manual that mentions this error message, and in case of call-with-new-thread situations proposes to do this 'add parameterize with another read/write waiter'). (An inconvenience is that the default read/write waiter isn't directly accessible, you would need (fluid-ref* [...] 1) to get access to it(*). Perhaps a primitive-read-waiter / primitive-write-waiter could be defined in Guile.

(*) doing (define default (current-read-waiter)) is not a proper solution, since whoever is loading the module might have set current-read-waiter to something else.

In ``fibers.scm` there is a comment >;; When a scheduler is passed explicitly, it could be there is no >;; current fiber; in that case the dynamic state probably doesn't >;; have the right right current-read-waiter / >;; current-write-waiter, so wrap the thunk. Could you test setting `current-read-waiter` and `current-write-waiter` to (its value outside the `run-fibers`) around the `call-with-new-thread` (or around the create-socket), to test the hypothesis if this kind of thing is going on? Assuming this is correct, first `suspend-current-task` should be modified to check if a current scheduler is defined before using it (and if not, do something like `(error "tried to suspend current task, but no current scheduler is defined")`, to make such situations clearer for the future. This shouldn't incur performance problems since a type check exists anyway. Likewise, `current-read-waiter` could be modified to make an error message like "Fiber's read waiter can only be used in a fiber context, consider setting current-read-waiter appropriately." Some text could be added to the manual that mentions this error message, and in case of `call-with-new-thread` situations proposes to do this 'add parameterize with another read/write waiter'). (An inconvenience is that the default read/write waiter isn't directly accessible, you would need (fluid-ref* [...] 1) to get access to it(*). Perhaps a `primitive-read-waiter` / `primitive-write-waiter` could be defined in Guile. (*) doing (define default (current-read-waiter)) is not a proper solution, since whoever is loading the module might have set `current-read-waiter` to something else.

emixa-d commented 2024年10月30日 20:19:02 +01:00 (Migrated from github.com)

I've adjusted title to what I think is going on. I don't think it's about sockets per se.

I've adjusted title to what I think is going on. I don't think it's about sockets per se.

abcdw commented 2024年12月25日 06:02:01 +01:00 (Migrated from github.com)

@emixa-d you are right. Setting them solves the issue (tried both options: around new thread and around create socket).

The following snippet of code produces the right result (Connection refused):

(define-module (nonblocking-socket)
 #:use-module (ice-9 threads)
 #:use-module (ice-9 suspendable-ports)
 #:use-module (fibers)
 #:export (create-socket))
(define* (create-socket
 #:key (port 1134))
 (define (make-default-socket family addr port)
 (let ((sock (socket PF_INET SOCK_STREAM 0)))
 ;; (fcntl sock F_SETFD FD_CLOEXEC)
 (fcntl sock F_SETFL (logior O_NONBLOCK (fcntl sock F_GETFL)))
 sock))
 (define tmp-socket
 (make-default-socket AF_INET INADDR_LOOPBACK 1234))
 (connect tmp-socket (make-socket-address AF_INET INADDR_LOOPBACK port))
 (close-port tmp-socket))
(define-public (tmp)
 (define prev-rw (current-read-waiter))
 (define prev-ww (current-write-waiter))
 (run-fibers
 (lambda ()
 (parameterize
 ((current-read-waiter prev-rw)
 (current-write-waiter prev-ww))
 (call-with-new-thread
 (lambda ()
 (create-socket))))
 (sleep 1)
 'yay)))

@emixa-d you are right. Setting them solves the issue (tried both options: around new thread and around create socket). The following snippet of code produces the right result (Connection refused): ``` (define-module (nonblocking-socket) #:use-module (ice-9 threads) #:use-module (ice-9 suspendable-ports) #:use-module (fibers) #:export (create-socket)) (define* (create-socket #:key (port 1134)) (define (make-default-socket family addr port) (let ((sock (socket PF_INET SOCK_STREAM 0))) ;; (fcntl sock F_SETFD FD_CLOEXEC) (fcntl sock F_SETFL (logior O_NONBLOCK (fcntl sock F_GETFL))) sock)) (define tmp-socket (make-default-socket AF_INET INADDR_LOOPBACK 1234)) (connect tmp-socket (make-socket-address AF_INET INADDR_LOOPBACK port)) (close-port tmp-socket)) (define-public (tmp) (define prev-rw (current-read-waiter)) (define prev-ww (current-write-waiter)) (run-fibers (lambda () (parameterize ((current-read-waiter prev-rw) (current-write-waiter prev-ww)) (call-with-new-thread (lambda () (create-socket)))) (sleep 1) 'yay))) ```