Phase 1.5: load config and initialize Server from ListenSpec list - #14
Merged
Conversation
…drop unused forward decl
There was a problem hiding this comment.
Pull request overview
Replaces the hardcoded single-listener startup with a config-driven flow: main() loads a config file (default config/default.conf or argv[1]), and Server now owns a vector of Socket* listeners — one per ListenSpec — plus a fd → ServerConfig* map for the future router. Socket gains a (host, port) constructor that uses getaddrinfo(AI_NUMERICHOST | AI_PASSIVE), sets SO_REUSEADDR, and follows a strict freeaddrinfo/close cleanup contract on every throw path. Legacy Socket(int) and Server(Socket&) are removed.
Changes:
Socketctor changed to(host, port)with numericgetaddrinfo,SO_REUSEADDR, and resource-cleanup contract.Serverctor now takesconst Config&, builds N listeners with partial-init RAII cleanup, and exposesfd_to_server_for routing.main()adds argv handling, default-path fallback, and a top-leveltry/catchprintingfatal: <msg>; new integration test script + fixture configs added.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main.cpp | New argv/default-path/try-catch driver; dev LEXER_DUMP/CONFIG_DUMP hooks preserved. |
| includes/socket.hpp | Replaces Socket(int) with Socket(const std::string&, int); adds <netdb.h>. |
| src/socket/socket.cpp | Implements new ctor using getaddrinfo, SO_REUSEADDR, non-blocking, bind with cleanup on every throw path. |
| includes/server.hpp | Adds config_, listeners_, fd_to_server_ members and Server(const Config&); removes Socket& member. |
| src/server/server.cpp | New ctor with partial-init RAII; run() and cleanup_sockets() use fd_to_server_ for listener detection; dtor deletes owned listeners. |
| tests/phase15_tests.sh | New bash integration harness covering argv, default path, multi-port, multi-server, duplicate, bad syntax. |
| config/test_multi_port.conf | Fixture: one server with two listen ports. |
| config/test_two_servers.conf | Fixture: two server blocks on distinct ports. |
| config/test_duplicate.conf | Fixture: duplicate listen for validator rejection test. |
| config/test_bad_syntax.conf | Fixture: malformed config for parser error test. |
| docs/.../phase-1-5-...-design.md | Design rationale, scope, and decisions log. |
| docs/.../phase-1-5-...-plan.md | Step-by-step implementation plan with tasks and TDD flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+58
to
69
| } catch (...) { | ||
| // Partial-init cleanup: a Socket ctor or startListening() threw. | ||
| // listeners_ contains only the ones that fully succeeded; delete | ||
| // each (their dtors close their fds). | ||
| for (std::size_t k = 0; k < listeners_.size(); ++k) | ||
| delete listeners_[k]; | ||
| listeners_.clear(); | ||
| fd_to_server_.clear(); | ||
| throw; | ||
| } | ||
|
|
||
| setup_signal_handlers(); |
Comment on lines
+107
to
+127
| # Task 4: default-path happy case — webserv with no arg listens on 8080 | ||
| ./webserv >/tmp/webserv.out 2>&1 & | ||
| PID=$! | ||
| sleep 1 | ||
| if ! kill -0 "$PID" 2>/dev/null; then | ||
| FAIL=$((FAIL+1)); FAILED_TESTS+=("task4_default_path: webserv exited: | ||
| $(cat /tmp/webserv.out)") | ||
| else | ||
| if lsof -iTCP:8080 -sTCP:LISTEN -P -n 2>/dev/null | grep -q webserv; then | ||
| PASS=$((PASS+1)) | ||
| else | ||
| FAIL=$((FAIL+1)); FAILED_TESTS+=("task4_default_path: not listening on 8080") | ||
| fi | ||
| kill -INT "$PID" 2>/dev/null; wait "$PID" 2>/dev/null | ||
| fi | ||
|
|
||
| # Task 6: one server, two listen directives | ||
| assert_listens_on_ports \ | ||
| "task6_multi_port_same_server" \ | ||
| "config/test_multi_port.conf" \ | ||
| 8080 8081 |
MukhammadIbrokhimov
force-pushed
the
feature/phase-1-5-load-config
branch
from
May 28, 2026 21:04
353d56a to
e3510af
Compare
run() used to walk listeners_ linearly to find which Socket* owned an incoming POLLIN fd. Cheap with one listener, painful by Phase 4 stress testing. New parallel map is kept in lock-step with fd_to_server_ in the ctor, so the existing is_listener check still works unchanged.
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.
Summary
Replaces the hardcoded single-socket startup in
main.cppwith a config-driven flow that opens N listening sockets (one perListenSpecincfg.servers()).Servernow owns the listeners and maintains anfd → ServerConfig*map that Phase 2.4's router will read.main: argv handling, default-path fallback (config/default.conf), single top-leveltry/catchprintingfatal: <msg>on anystd::exception. ExistingLEXER_DUMP/CONFIG_DUMPdev hooks preserved per the kanban Phase-4 cleanup schedule.Socket: new ctorSocket(const std::string& host, int port)resolves the host viagetaddrinfo(AI_NUMERICHOST | AI_PASSIVE)(no DNS, no blocking), setsSO_REUSEADDRbeforebind, and follows a strict cleanup contract — every throw path runsfreeaddrinfo(res)and::close(fd)before propagating. Port range guarded (1-65535).Server: new ctorServer(const Config&)walkscfg.servers(), allocates oneSocket*perListenSpec, populatesstd::map<int, const ServerConfig*> fd_to_server_. Partial-init RAII pattern: the construction loop is wrapped intry { } catch(...) { delete partial state; throw; }to handle the pre-C++11 case where ctor exceptions skip the destructor. Signal handlers install as the last ctor step so the invariant "handlers installed iff Server fully constructed" holds.6923124):Socket(int port)andServer(Socket&)no longer exist. Staged refactor kept every interim commit buildable.Subject-compliance notes:
getaddrinfo,freeaddrinfo,setsockopt,fcntl,socket,bind,listen,htons,close) appear in the allowed-functions list on subject p.6.fcntlrestriction respected: onlyF_SETFL+O_NONBLOCKused.(host, port)across server blocks is rejected at validate-time (rule already lived inConfig::validatefrom Phase 1.4).sigactionis used inServer::setup_signal_handlersbut onlysignalis on the subject's allowed list. Not introduced by this PR; flagging for awareness.Test plan
Integration tests live in
tests/phase15_tests.sh(7 cases, all passing). All kanban Phase 1.5 DoD items verified:./webservloadsconfig/default.confand listens on 8080./webserv config/test_two_servers.confbinds 8090 AND 9090fatal: cannot open …+ exit 1test_duplicate.conf): rejected at validation, exit 1, no bind attemptedtest_bad_syntax.conf): rejected at parse, exit 1, no crashfatal: <line>:<col>: …, exit 1, no segfault./webserv default.conf foo bar): usage + exit 1fatal: bind failed on 0.0.0.0:8080, exit 1lsofconfirms)make reclean under-Wall -Wextra -Werror -std=c++98, zero warningsRun locally:
Expected:
Phase 1.5: 7 passed, 0 failed.Known follow-ups (out of scope for this PR)
Server::run()swallowsPOLLHUP|POLLERRon listener fds. Pre-existing pattern from Phase 1.2; with N listeners worth defensive logging + clean shutdown in a Phase 2 polish PR.Server::run()'s accept block to map fd back to the owningSocket*. O(log N) lookup via a sibling map would be cleaner; trivial perf, deferred to Phase 2.4 when the router work touches this code anyway.Phase 2 readiness
fd_to_server_is now populated and ready for the request router (Phase 2.4) to dispatch by listener fd. Theconst Config&borrow is documented with an explicit immutability invariant on the relevant member declaration.