Skip to content

Commit edca32a

Browse files
youknowoneyouknowone
authored
Fix ssl shutdown (#6871)
* Fix ssl shutdown * Fix thread
1 parent 93274d3 commit edca32a

File tree

3 files changed

+248
-63
lines changed

3 files changed

+248
-63
lines changed

‎crates/stdlib/src/openssl.rs‎

Lines changed: 101 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2805,49 +2805,120 @@ mod _ssl {
28052805
let stream = self.connection.read();
28062806
let ssl_ptr = stream.ssl().as_ptr();
28072807

2808-
// Perform SSL shutdown - may need to be called twice:
2809-
// 1st call: sends close-notify, returns 0
2810-
// 2nd call: reads peer's close-notify, returns 1
2811-
let mut ret = unsafe { sys::SSL_shutdown(ssl_ptr) };
2812-
2813-
// If ret == 0, try once more to complete the bidirectional shutdown
2814-
// This handles the case where peer's close-notify is already available
2815-
if ret == 0 {
2816-
ret = unsafe { sys::SSL_shutdown(ssl_ptr) };
2808+
// BIO mode: just try shutdown once and raise SSLWantReadError if needed
2809+
if stream.is_bio() {
2810+
let ret = unsafe { sys::SSL_shutdown(ssl_ptr) };
2811+
if ret < 0 {
2812+
let err = unsafe { sys::SSL_get_error(ssl_ptr, ret) };
2813+
if err == sys::SSL_ERROR_WANT_READ {
2814+
return Err(create_ssl_want_read_error(vm).upcast());
2815+
} else if err == sys::SSL_ERROR_WANT_WRITE {
2816+
return Err(create_ssl_want_write_error(vm).upcast());
2817+
} else {
2818+
return Err(new_ssl_error(
2819+
vm,
2820+
format!("SSL shutdown failed: error code {}", err),
2821+
));
2822+
}
2823+
} else if ret == 0 {
2824+
// Sent close-notify, waiting for peer's - raise SSLWantReadError
2825+
return Err(create_ssl_want_read_error(vm).upcast());
2826+
}
2827+
return Ok(None);
28172828
}
28182829

2819-
if ret < 0 {
2820-
// Error occurred
2830+
// Socket mode: loop with select to wait for peer's close-notify
2831+
let socket_stream = stream.get_ref().expect("get_ref() failed for socket mode");
2832+
let deadline = socket_stream.timeout_deadline();
2833+
2834+
// Track how many times we've seen ret == 0 (max 2 tries)
2835+
let mut zeros = 0;
2836+
2837+
loop {
2838+
let ret = unsafe { sys::SSL_shutdown(ssl_ptr) };
2839+
2840+
// ret > 0: complete shutdown
2841+
if ret > 0 {
2842+
break;
2843+
}
2844+
2845+
// ret == 0: sent our close-notify, need to receive peer's
2846+
if ret == 0 {
2847+
zeros += 1;
2848+
if zeros > 1 {
2849+
// Already tried twice, break out (legacy behavior)
2850+
break;
2851+
}
2852+
// Wait briefly for peer's close_notify before retrying
2853+
match socket_stream.select(SslNeeds::Read, &deadline) {
2854+
SelectRet::TimedOut => {
2855+
return Err(vm.new_exception_msg(
2856+
vm.ctx.exceptions.timeout_error.to_owned(),
2857+
"The read operation timed out".to_owned(),
2858+
));
2859+
}
2860+
SelectRet::Closed => {
2861+
return Err(socket_closed_error(vm));
2862+
}
2863+
SelectRet::Nonblocking => {
2864+
// Non-blocking socket: return SSLWantReadError
2865+
return Err(create_ssl_want_read_error(vm).upcast());
2866+
}
2867+
SelectRet::Ok => {
2868+
// Data available, continue to retry
2869+
}
2870+
}
2871+
continue;
2872+
}
2873+
2874+
// ret < 0: error or would-block
28212875
let err = unsafe { sys::SSL_get_error(ssl_ptr, ret) };
28222876

2823-
if err == sys::SSL_ERROR_WANT_READ {
2824-
return Err(create_ssl_want_read_error(vm).upcast());
2877+
let needs = if err == sys::SSL_ERROR_WANT_READ {
2878+
SslNeeds::Read
28252879
} else if err == sys::SSL_ERROR_WANT_WRITE {
2826-
return Err(create_ssl_want_write_error(vm).upcast());
2880+
SslNeeds::Write
28272881
} else {
2882+
// Real error
28282883
return Err(new_ssl_error(
28292884
vm,
28302885
format!("SSL shutdown failed: error code {}", err),
28312886
));
2832-
}
2833-
} else if ret == 0 {
2834-
// Still waiting for peer's close-notify after retry
2835-
// In BIO mode, raise SSLWantReadError
2836-
if stream.is_bio() {
2837-
return Err(create_ssl_want_read_error(vm).upcast());
2838-
}
2839-
}
2887+
};
28402888

2841-
// BIO mode doesn't have an underlying socket to return
2842-
if stream.is_bio() {
2843-
return Ok(None);
2889+
// Wait on the socket
2890+
match socket_stream.select(needs, &deadline) {
2891+
SelectRet::TimedOut => {
2892+
let msg = if err == sys::SSL_ERROR_WANT_READ {
2893+
"The read operation timed out"
2894+
} else {
2895+
"The write operation timed out"
2896+
};
2897+
return Err(vm.new_exception_msg(
2898+
vm.ctx.exceptions.timeout_error.to_owned(),
2899+
msg.to_owned(),
2900+
));
2901+
}
2902+
SelectRet::Closed => {
2903+
return Err(socket_closed_error(vm));
2904+
}
2905+
SelectRet::Nonblocking => {
2906+
// Non-blocking socket, raise SSLWantReadError/SSLWantWriteError
2907+
if err == sys::SSL_ERROR_WANT_READ {
2908+
return Err(create_ssl_want_read_error(vm).upcast());
2909+
} else {
2910+
return Err(create_ssl_want_write_error(vm).upcast());
2911+
}
2912+
}
2913+
SelectRet::Ok => {
2914+
// Socket is ready, retry shutdown
2915+
continue;
2916+
}
2917+
}
28442918
}
28452919

2846-
// Return the underlying socket for socket mode
2847-
let socket = stream
2848-
.get_ref()
2849-
.expect("unwrap() called on bio mode; should only be called in socket mode");
2850-
Ok(Some(socket.0.clone()))
2920+
// Return the underlying socket
2921+
Ok(Some(socket_stream.0.clone()))
28512922
}
28522923

28532924
#[cfg(osslconf = "OPENSSL_NO_COMP")]

‎crates/stdlib/src/ssl.rs‎

Lines changed: 103 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -4119,45 +4119,115 @@ mod _ssl {
41194119
peer_closed = true;
41204120
}
41214121
} else if let Some(timeout) = timeout_mode {
4122-
// All socket modes (blocking, timeout, non-blocking):
4123-
// Return immediately after sending our close_notify.
4124-
//
4125-
// This matches CPython/OpenSSL behavior where SSL_shutdown()
4126-
// returns after sending close_notify, allowing the app to
4127-
// close the socket without waiting for peer's close_notify.
4128-
//
4129-
// Waiting for peer's close_notify can cause deadlock with
4130-
// asyncore-based servers where both sides wait for the other's
4131-
// close_notify before closing the connection.
4132-
4133-
// Ensure all pending TLS data is sent before returning
4134-
// This prevents data loss when rustls drains its buffer
4135-
// but the socket couldn't accept all data immediately
4136-
drop(conn_guard);
4137-
4138-
// Respect socket timeout settings for flushing pending TLS data
41394122
match timeout {
41404123
Some(0.0) => {
4141-
// Non-blocking: best-effort flush, ignore errors
4142-
// to avoid deadlock with asyncore-based servers
4124+
// Non-blocking: return immediately after sending close_notify.
4125+
// Don't wait for peer's close_notify to avoid blocking.
4126+
drop(conn_guard);
4127+
// Best-effort flush; WouldBlock is expected in non-blocking mode.
4128+
// Other errors indicate close_notify may not have been sent,
4129+
// but we still complete shutdown to avoid inconsistent state.
41434130
let _ = self.flush_pending_tls_output(vm, None);
4131+
*self.shutdown_state.lock() = ShutdownState::Completed;
4132+
*self.connection.lock() = None;
4133+
return Ok(self.sock.clone());
41444134
}
4145-
Some(_t) => {
4146-
// Timeout mode: use flush with socket's timeout
4147-
// Errors (including timeout) are propagated to caller
4148-
self.flush_pending_tls_output(vm, None)?;
4149-
}
4150-
None => {
4151-
// Blocking mode: wait until all pending data is sent
4152-
self.blocking_flush_all_pending(vm)?;
4135+
_ => {
4136+
// Blocking or timeout mode: wait for peer's close_notify.
4137+
// This is proper TLS shutdown - we should receive peer's
4138+
// close_notify before closing the connection.
4139+
drop(conn_guard);
4140+
4141+
// Flush our close_notify first
4142+
if timeout.is_none() {
4143+
self.blocking_flush_all_pending(vm)?;
4144+
} else {
4145+
self.flush_pending_tls_output(vm, None)?;
4146+
}
4147+
4148+
// Calculate deadline for timeout mode
4149+
let deadline = timeout.map(|t| {
4150+
std::time::Instant::now() + std::time::Duration::from_secs_f64(t)
4151+
});
4152+
4153+
// Wait for peer's close_notify
4154+
loop {
4155+
// Re-acquire connection lock for each iteration
4156+
let mut conn_guard = self.connection.lock();
4157+
let conn = match conn_guard.as_mut() {
4158+
Some(c) => c,
4159+
None => break, // Connection already closed
4160+
};
4161+
4162+
// Check if peer already sent close_notify
4163+
if self.check_peer_closed(conn, vm)? {
4164+
break;
4165+
}
4166+
4167+
drop(conn_guard);
4168+
4169+
// Check timeout
4170+
let remaining_timeout = if let Some(dl) = deadline {
4171+
let now = std::time::Instant::now();
4172+
if now >= dl {
4173+
// Timeout reached - raise TimeoutError
4174+
return Err(vm.new_exception_msg(
4175+
vm.ctx.exceptions.timeout_error.to_owned(),
4176+
"The read operation timed out".to_owned(),
4177+
));
4178+
}
4179+
Some(dl - now)
4180+
} else {
4181+
None // Blocking mode: no timeout
4182+
};
4183+
4184+
// Wait for socket to be readable
4185+
let timed_out = self.sock_wait_for_io_with_timeout(
4186+
SelectKind::Read,
4187+
remaining_timeout,
4188+
vm,
4189+
)?;
4190+
4191+
if timed_out {
4192+
// Timeout waiting for peer's close_notify
4193+
// Raise TimeoutError
4194+
return Err(vm.new_exception_msg(
4195+
vm.ctx.exceptions.timeout_error.to_owned(),
4196+
"The read operation timed out".to_owned(),
4197+
));
4198+
}
4199+
4200+
// Try to read data from socket
4201+
let mut conn_guard = self.connection.lock();
4202+
let conn = match conn_guard.as_mut() {
4203+
Some(c) => c,
4204+
None => break,
4205+
};
4206+
4207+
// Read and process any incoming TLS data
4208+
match self.try_read_close_notify(conn, vm) {
4209+
Ok(closed) => {
4210+
if closed {
4211+
break;
4212+
}
4213+
// Check again after processing
4214+
if self.check_peer_closed(conn, vm)? {
4215+
break;
4216+
}
4217+
}
4218+
Err(_) => {
4219+
// Socket error - peer likely closed connection
4220+
break;
4221+
}
4222+
}
4223+
}
4224+
4225+
// Shutdown complete
4226+
*self.shutdown_state.lock() = ShutdownState::Completed;
4227+
*self.connection.lock() = None;
4228+
return Ok(self.sock.clone());
41534229
}
41544230
}
4155-
4156-
// Set shutdown_state first to ensure atomic visibility
4157-
// This prevents read/write race conditions during shutdown
4158-
*self.shutdown_state.lock() = ShutdownState::Completed;
4159-
*self.connection.lock() = None;
4160-
return Ok(self.sock.clone());
41614231
}
41624232

41634233
// Step 3: Check again if peer has sent close_notify (non-blocking/BIO mode only)

‎crates/vm/src/stdlib/thread.rs‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -936,6 +936,50 @@ pub(crate) mod _thread {
936936
true
937937
});
938938
}
939+
940+
// Clean up shutdown_handles as well.
941+
// This is critical to prevent _shutdown() from waiting on threads
942+
// that don't exist in the child process after fork.
943+
if let Some(mut handles) = vm.state.shutdown_handles.try_lock() {
944+
// Mark all non-current threads as done in shutdown_handles
945+
handles.retain(|(inner_weak, done_event_weak): &ShutdownEntry| {
946+
let Some(inner) = inner_weak.upgrade() else {
947+
return false; // Remove dead entries
948+
};
949+
let Some(done_event) = done_event_weak.upgrade() else {
950+
return false;
951+
};
952+
953+
// Try to lock the inner state - skip if we can't
954+
let Some(mut inner_guard) = inner.try_lock() else {
955+
return false;
956+
};
957+
958+
// Skip current thread
959+
if inner_guard.ident == current_ident {
960+
return true;
961+
}
962+
963+
// Keep handles for threads that have not been started yet.
964+
// They are safe to start in the child process.
965+
if inner_guard.state == ThreadHandleState::NotStarted {
966+
return true;
967+
}
968+
969+
// Mark as done so _shutdown() won't wait on it
970+
inner_guard.state = ThreadHandleState::Done;
971+
drop(inner_guard);
972+
973+
// Notify waiters
974+
let (lock, cvar) = &*done_event;
975+
if let Some(mut done) = lock.try_lock() {
976+
*done = true;
977+
cvar.notify_all();
978+
}
979+
980+
false // Remove from shutdown_handles - these threads don't exist in child
981+
});
982+
}
939983
}
940984

941985
// Thread handle state enum

0 commit comments

Comments
 (0)