From 899aa247565bce1ff9564959f59d3e0b07b8a710 Mon Sep 17 00:00:00 2001 From: Matthew Flatt Date: Tue, 17 Dec 2019 07:06:49 -0700 Subject: [PATCH] cs: avoid problem waking up on TCP connect completion Maybe there's a problem in `rktio_poll_add_connect` that I just can't see, or maybe it's a Mac OS bug, but `rktio_poll_add_connect` doesn't seem to reliably wake up the process when the TCP connection becomes ready. Traditional Racket happens to avoid the problem by registering the connection file descriptor with the semaphore table; doing that for Racket CS also avoids the problem there. --- racket/src/io/demo.rkt | 2 -- racket/src/io/network/evt.rkt | 9 +------ racket/src/io/network/tcp-connect.rkt | 38 ++++++++++++++++++++++----- racket/src/racket/src/network.c | 6 ++++- 4 files changed, 38 insertions(+), 17 deletions(-) diff --git a/racket/src/io/demo.rkt b/racket/src/io/demo.rkt index 7294ad6a0b..e1571974ca 100644 --- a/racket/src/io/demo.rkt +++ b/racket/src/io/demo.rkt @@ -416,7 +416,6 @@ (test (void) (file-position out 10)) (test #"hola!!\0\0\0\0" (get-output-bytes out))) -(log-error "start") (let () (define-values (i o) (make-pipe)) (port-count-lines! i) @@ -444,7 +443,6 @@ (write-bytes #"!" o) (test '(3 1 8) (next-location o)) -(log-error "here") (test #"x\r" (read-bytes 2 i)) (test '(3 0 7) (next-location i)) (test #"\n!" (read-bytes 2 i)) diff --git a/racket/src/io/network/evt.rkt b/racket/src/io/network/evt.rkt index 2896c19a6d..77e2522768 100644 --- a/racket/src/io/network/evt.rkt +++ b/racket/src/io/network/evt.rkt @@ -14,13 +14,6 @@ [((rktio-evt-poll self)) (values (list self) #f)] [else - (define sched-info (poll-ctx-sched-info poll-ctx)) - (when sched-info - ;; Cooperate with the sandman by registering a function that - ;; takes a poll set and adds to it: - (schedule-info-current-exts sched-info - (sandman-add-poll-set-adder - (schedule-info-current-exts sched-info) - (rktio-evt-add-to-poll-set self)))) + (sandman-poll-ctx-add-poll-set-adder! poll-ctx (rktio-evt-add-to-poll-set self)) (values #f self)]))) #:authentic) diff --git a/racket/src/io/network/tcp-connect.rkt b/racket/src/io/network/tcp-connect.rkt index c07168b071..bec13906e2 100644 --- a/racket/src/io/network/tcp-connect.rkt +++ b/racket/src/io/network/tcp-connect.rkt @@ -6,6 +6,7 @@ "../host/error.rkt" "../security/main.rkt" "../format/main.rkt" + "../sandman/ltps.rkt" "tcp-port.rkt" "port-number.rkt" "address.rkt" @@ -15,6 +16,10 @@ (provide tcp-connect tcp-connect/enable-break) +(struct connect-progress (conn trying-fd) + #:mutable + #:authentic) + (define/who (tcp-connect hostname port-no [local-hostname #f] [local-port-no #f]) (do-tcp-connect who hostname port-no local-hostname local-port-no)) @@ -66,15 +71,17 @@ (raise-connect-error local-addr "local host not found" local-hostname local-port-no)] [else (call-with-resource - (box (rktio_start_connect rktio remote-addr local-addr)) + (connect-progress (rktio_start_connect rktio remote-addr local-addr) + #f) ;; in atomic mode - (lambda (conn-box) - (define conn (unbox conn-box)) + (lambda (conn-prog) + (remove-trying-fd! conn-prog) + (define conn (connect-progress-conn conn-prog)) (when conn (rktio_connect_stop rktio conn))) ;; in atomic mode - (lambda (conn-box) - (define conn (unbox conn-box)) + (lambda (conn-prog) + (define conn (connect-progress-conn conn-prog)) (cond [(rktio-error? conn) (raise-connect-error conn)] @@ -83,6 +90,7 @@ (cond [(eqv? (rktio_poll_connect_ready rktio conn) RKTIO_POLL_NOT_READY) + (init-trying-fd! conn-prog) (end-atomic) ((if enable-break? sync/enable-break sync) (rktio-evt (lambda () @@ -93,6 +101,7 @@ (start-atomic) (loop)] [else + (remove-trying-fd! conn-prog) (check-current-custodian who) (define fd (rktio_connect_finish rktio conn)) (cond @@ -102,8 +111,25 @@ (loop)] [else ;; other errors imply that `conn` is destroyed - (set-box! conn-box #f) + (set-connect-progress-conn! conn-prog #f) (raise-connect-error fd)])] [else (define name (string->immutable-string hostname)) (open-input-output-tcp fd name)])]))])))])))]))))) + +;; in atomic mode +(define (init-trying-fd! conn-prog) + (unless (connect-progress-trying-fd conn-prog) + ;; Even though we don't use the semaphore for the registered file + ;; descriptor, registering it seems to avoid a problem where + ;; `rktio_poll_add_connect` doesn't work, at least on Mac OS. + (define fd (rktio_connect_trying rktio (connect-progress-conn conn-prog))) + (set-connect-progress-trying-fd! conn-prog fd) + (void (fd-semaphore-update! fd 'write)))) + +;; in atomic mode +(define (remove-trying-fd! conn-prog) + (define fd (connect-progress-trying-fd conn-prog)) + (when fd + (fd-semaphore-update! fd 'remove) + (set-connect-progress-trying-fd! conn-prog #f))) diff --git a/racket/src/racket/src/network.c b/racket/src/racket/src/network.c index 4476aef2a2..fa26d31ce0 100644 --- a/racket/src/racket/src/network.c +++ b/racket/src/racket/src/network.c @@ -893,8 +893,12 @@ static int tcp_check_connect(Connect_Progress_Data *pd, Scheme_Schedule_Info *si if (rktio_poll_connect_ready(scheme_rktio, pd->connect)) return 1; - if (pd->trying_s) + if (pd->trying_s) { + /* Even though we don't currently use the semaphore for blocking, + registering with the semaphore table seems to avoid a problem + with `rktio_poll_add_connect` not working, at least on Mac OS. */ check_fd_sema(pd->trying_s, MZFD_CREATE_WRITE, sinfo, NULL); + } return 0; }