Fix openssl build and shared ssl/error.rs by youknowone · Pull Request #6456 · RustPython/RustPython
593-606: LGTM - Manual ex_data cleanup is correctly implemented.
The cleanup_sni_ex_data function properly handles the lifecycle of SniCallbackData:
- Checks for null before freeing (safe for multiple calls)
- Clears the ex_data pointer to prevent double-free
- Called explicitly after handshake to release references promptly
712-727: LGTM - Intentional no-op callback is well-documented.
The comment clearly explains the deadlock concerns (referencing CPython bpo-43577) and why this callback is intentionally empty. This is a reasonable design choice that prioritizes stability over the feature.
2188-2192: LGTM - Simple and correct pending() implementation.
The method correctly exposes OpenSSL's SSL_pending function, which returns the number of bytes available for immediate read. Implementation is straightforward and matches CPython's behavior.
3176-3180: LGTM - Unified error creation helper.
The new_ssl_error helper provides a consistent way to create SSL errors as proper OSError subtypes, which aligns with CPython's behavior. This centralizes the error creation pattern used throughout the module.
2406-2483: LGTM - Comprehensive handshake cleanup across all exit paths.
The do_handshake implementation correctly calls cleanup_sni_ex_data on all exit paths (success, timeout, closed, error). The HandshakeVmGuard RAII pattern ensures the thread-local state is properly managed. The safety comments appropriately document the lifetime requirements.
1704-1720: LGTM - Weak reference prevents reference cycle.
Using PyWeak for ssl_socket_weak correctly breaks the reference cycle that would otherwise occur: PySslSocket → SslStream → SSL → ex_data → SniCallbackData → back to PySslSocket. The implementation properly minimizes lock contention by checking has_sni_callback before doing the more expensive weak reference creation.
554-562: LGTM - Correctly avoids deadlock during handshake callbacks.
The function properly handles the scenario where set_context might be called from within an SNI callback during handshake (when the connection lock is already held). By checking the thread-local HANDSHAKE_SSL_PTR first, it avoids attempting to re-acquire the lock.