Draft
Conversation
Allow accepting new sessions from multiple threads. Made server sockets non-blocking.
Tests accepting connections with multiple threads with different timeouts.
There was a problem hiding this comment.
Pull request overview
This PR enables parallel nc_accept() usage by making listening sockets non-blocking and refactoring the accept/poll logic to be safe across multiple server accept threads. It also adds a dedicated concurrency-focused test and bumps the library micro version.
Changes:
- Refactor server-side accept to poll/accept without per-bind mutexes by relying on non-blocking listening sockets.
- Remove
bind_lock/pollinstate from internal bind structures and update call-home accept accordingly. - Add
test_server_threadto validate parallel accept behavior and fix minor TLS errno checks.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_server_thread.c |
Adds a new cmocka test exercising parallel server accepts with multiple server/client threads. |
tests/CMakeLists.txt |
Registers the new test_server_thread test target. |
src/session_server.c |
Makes listening sockets non-blocking and introduces new pollfd-based concurrent accept helpers. |
src/session_p.h |
Removes obsolete bind fields/locks and updates internal accept API declarations/comments. |
src/session_client.c |
Updates call-home accept to use the new accept helper and removes the client CH bind lock. |
src/session_client_tls.c |
Minor TLS handshake loop cleanup (uses the correct socket variable). |
src/session_mbedtls.c |
Fixes errno checks (assignment → comparison) for WANT_READ/WANT_WRITE handling. |
src/server_config.c |
Removes endpoint bind-lock initialization/destruction now that it’s no longer used. |
CMakeLists.txt |
Bumps micro version/soversion to reflect internal changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+603
to
606
| nc_sock_host_unix(int acc_sock_fd, char **host) | ||
| { | ||
| char *sun_path; | ||
| struct sockaddr_storage saddr; |
Comment on lines
+736
to
+737
| /* UNIX socket, get the socket path for logging */ | ||
| unix_sockpath = nc_server_unix_get_socket_path(endpt); |
| return -1; | ||
| } | ||
|
|
||
| /* sucessfully accepted a connection! */ |
Comment on lines
+105
to
+109
| nc_client_set_thread_context(state->client_thr_ctx); | ||
|
|
||
| pthread_barrier_wait(&state->start_barrier); | ||
|
|
||
| session = nc_connect_ssh("127.0.0.1", TEST_PORT, NULL); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Truly allow accepting new sessions from multiple threads.