QUIC: Immediate-Mode Polling API #24388
Conversation
…r inter-thread notification
|
|
||
| ql->domain = ctx.qd; | ||
| ql->engine = ctx.qd->engine; | ||
| ql->mutex = ctx.qd->mutex; |
There was a problem hiding this comment.
the definition of ql->mutex is gated on OPENSSL_THREADS. A build configured with no-threads will break here as ->mutex will be undefined
| * Make socket close-on-exec unless this was already done above at socket | ||
| * creation time. | ||
| */ | ||
| if (!set_cloexec(fd)) { |
There was a problem hiding this comment.
I'm not sure I understand this comment. The socket is created by this function, so this is socket creation time, no?
| goto err; | ||
|
|
||
| /* Make both sides of the connection non-blocking. */ | ||
| if (!BIO_socket_nbio(rfd, 1)) |
There was a problem hiding this comment.
If we're doing non-blocking io on a tcp socket here, wouldn't it be simpler to just use a dgram pair?
ssl/quic/quic_reactor.c
Outdated
| */ | ||
| if (rtor->have_notifier && rtor->signalled_notifier) { | ||
| if (rtor->cur_blocking_waiters == 0) { | ||
| ossl_rio_notifier_unsignal(&rtor->notifier); |
There was a problem hiding this comment.
I can't put my finger on the exact reason why, but having to do un-signaling here feels wrong. need to think about this some more
| type |= SOCK_NONBLOCK; | ||
| # endif | ||
|
|
||
| #if defined(OPENSSL_SYS_UNIX) && defined(AF_UNIX) |
There was a problem hiding this comment.
According to this:
https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/socketpair.2.html
socketpair is only implemented in macos for SYS_UNIX, but I think our macosx builds define OPENSSL_SYS_MACOSX rather than OPENSSL_SYS_UNIX. This should likely be expanded to support setting AF_UNIX for that platform
Fixes openssl/project#299.
Built on #24257.
Review #24257 first.