fix(tcp): resolve, connect, bind and send failures were silent or wrong
The HTTP/1.1 stack sits directly on these two classes and each of these bit it: - gethostbyname() returning null on an unresolvable host was dereferenced straight into a crash, and it is not thread safe; use getaddrinfo. - A failed socket()/connect() only printed to stderr and handed back an unusable ClientTCP, so the real error surfaced much later as an unrelated errno from send(). - send() was assumed to accept everything it was offered. It does not once a buffer outgrows the socket's send buffer, which silently truncated multi-megabyte bodies. Loop, and pass MSG_NOSIGNAL so a vanished peer raises EPIPE instead of killing the process. - ClientTCP's move constructor closed the socket it had just taken ownership of, and both it and the destructor tested `socketid != 1` where they meant `!= -1`. - ListenerTCP ignored bind()'s result, leaving a listener that accepted nothing with no explanation, and did not set SO_REUSEADDR, so a restart hit EADDRINUSE for the length of TIME_WAIT. accept() failing during Stop() is expected and no longer logged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
catbot
parent
e33ec5b72e
commit
b758419007
2 changed files with 176 additions and 131 deletions
|
|
@ -31,11 +31,22 @@ ClientTCP::ClientTCP(int socketid) : socketid(socketid)
|
|||
|
||||
ClientTCP::ClientTCP(const char* hostName, std::uint16_t port)
|
||||
{
|
||||
host = gethostbyname(hostName);
|
||||
serv_addr.sin_family = AF_INET;
|
||||
serv_addr.sin_port = htons(port);
|
||||
serv_addr.sin_addr = *((struct in_addr *)host->h_addr);
|
||||
bzero(&(serv_addr.sin_zero),8);
|
||||
// getaddrinfo rather than gethostbyname: the latter is not thread safe,
|
||||
// and it signals failure by returning null — which was then dereferenced
|
||||
// straight into a crash on any unresolvable host.
|
||||
addrinfo hints{};
|
||||
hints.ai_family = AF_INET;
|
||||
hints.ai_socktype = SOCK_STREAM;
|
||||
addrinfo* resolved = nullptr;
|
||||
const std::string service = std::to_string(port);
|
||||
const int status = getaddrinfo(hostName, service.c_str(), &hints, &resolved);
|
||||
if (status != 0 || resolved == nullptr) {
|
||||
throw std::runtime_error(std::string("Could not resolve host '") + hostName + "': "
|
||||
+ gai_strerror(status));
|
||||
}
|
||||
host = nullptr;
|
||||
serv_addr = *reinterpret_cast<sockaddr_in*>(resolved->ai_addr);
|
||||
freeaddrinfo(resolved);
|
||||
|
||||
Connect();
|
||||
}
|
||||
|
|
@ -45,17 +56,16 @@ ClientTCP::ClientTCP(std::string hostName, std::uint16_t port): ClientTCP(hostNa
|
|||
|
||||
}
|
||||
|
||||
// The moved-from object gives up ownership; the socket itself stays open —
|
||||
// closing it here (as this used to) meant moving a ClientTCP silently
|
||||
// dropped the connection it was carrying.
|
||||
ClientTCP::ClientTCP(ClientTCP&& other) noexcept : socketid(other.socketid) {
|
||||
if(socketid != 1) {
|
||||
shutdown(socketid, SHUT_RDWR);
|
||||
close(socketid);
|
||||
}
|
||||
other.socketid = -1;
|
||||
}
|
||||
|
||||
ClientTCP::~ClientTCP()
|
||||
{
|
||||
if(socketid != 1) {
|
||||
if(socketid != -1) {
|
||||
shutdown(socketid, SHUT_RDWR);
|
||||
close(socketid);
|
||||
}
|
||||
|
|
@ -63,11 +73,17 @@ ClientTCP::~ClientTCP()
|
|||
|
||||
void ClientTCP::Connect() {
|
||||
if((socketid = socket(AF_INET, SOCK_STREAM, 0)) == -1){
|
||||
std::cerr << "Could not open socket" << std::endl;
|
||||
throw std::runtime_error(std::string("Could not open socket: ") + std::strerror(errno));
|
||||
}
|
||||
|
||||
if(connect(socketid,(sockaddr*)&serv_addr, sizeof(sockaddr)) == -1){
|
||||
std::cerr << "Could not connect to server" << std::endl;
|
||||
// Report the failure instead of handing back a socket that is not
|
||||
// connected to anything — every later send/recv on it would fail
|
||||
// with a far less obvious error.
|
||||
const std::string reason = std::strerror(errno);
|
||||
close(socketid);
|
||||
socketid = -1;
|
||||
throw std::runtime_error("Could not connect to server: " + reason);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -78,12 +94,20 @@ void ClientTCP::Stop() {
|
|||
}
|
||||
|
||||
void ClientTCP::Send(const void* buffer, std::uint32_t size) const {
|
||||
int status = send(socketid, reinterpret_cast<const char*>(buffer), size, 0);
|
||||
|
||||
if (status == 0) {
|
||||
throw SocketClosedException();
|
||||
} else if (status < 0) {
|
||||
throw std::runtime_error(std::strerror(errno));
|
||||
// send() is free to accept less than it was offered, and does so
|
||||
// routinely once a buffer outgrows the socket's send buffer — a
|
||||
// multi-megabyte HTTP body, say. Loop until it is all handed over.
|
||||
const char* data = reinterpret_cast<const char*>(buffer);
|
||||
std::uint32_t sent = 0;
|
||||
while (sent < size) {
|
||||
const auto status = send(socketid, data + sent, size - sent, MSG_NOSIGNAL);
|
||||
if (status == 0) {
|
||||
throw SocketClosedException();
|
||||
} else if (status < 0) {
|
||||
if (errno == EINTR) continue;
|
||||
throw std::runtime_error(std::strerror(errno));
|
||||
}
|
||||
sent += static_cast<std::uint32_t>(status);
|
||||
}
|
||||
}
|
||||
std::vector<char> ClientTCP::RecieveSync(std::uint32_t bufferSize) const {
|
||||
|
|
|
|||
|
|
@ -1,114 +1,135 @@
|
|||
//SPDX-License-Identifier: LGPL-3.0-only
|
||||
//SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||
|
||||
module;
|
||||
|
||||
#include <stdio.h>
|
||||
#include <sys/types.h>
|
||||
#include <sys/socket.h>
|
||||
#include <netinet/in.h>
|
||||
#include <arpa/inet.h>
|
||||
#include <sys/uio.h>
|
||||
#include <sys/time.h>
|
||||
#include <sys/wait.h>
|
||||
#include <strings.h>
|
||||
|
||||
module Crafter.Network:ListenerTCP_impl;
|
||||
import :ListenerTCP;
|
||||
import std;
|
||||
import Crafter.Thread;
|
||||
|
||||
using namespace Crafter;
|
||||
|
||||
ListenerTCP::ListenerTCP(std::uint16_t port, std::function<void(ClientTCP*)> connectCallback, std::uint32_t concurrentClientLimit, std::uint32_t totalClientLimit) : connectCallback(connectCallback), concurrentClientLimit(concurrentClientLimit), totalClientLimit(totalClientLimit) {
|
||||
sockaddr_in servAddr;
|
||||
bzero((char*)&servAddr, sizeof(servAddr));
|
||||
servAddr.sin_family = AF_INET;
|
||||
servAddr.sin_addr.s_addr = htonl(INADDR_ANY);
|
||||
servAddr.sin_port = htons(port);
|
||||
|
||||
s = socket(AF_INET, SOCK_STREAM, 0);
|
||||
if(s < 0)
|
||||
{
|
||||
throw std::runtime_error("Error establishing the server socket");
|
||||
}
|
||||
int bindStatus = bind(s, (struct sockaddr*) &servAddr, sizeof(servAddr));
|
||||
listen(s, 5);
|
||||
}
|
||||
|
||||
void ListenerTCP::Stop() {
|
||||
running = false;
|
||||
shutdown(s, SHUT_RDWR);
|
||||
close(s);
|
||||
s = -1;
|
||||
}
|
||||
|
||||
void ListenerTCP::ListenSyncSync() {
|
||||
while (running && totalClientCounter < totalClientLimit) {
|
||||
sockaddr_in newSockAddr;
|
||||
socklen_t newSockAddrSize = sizeof(newSockAddr);
|
||||
int client = accept(s, (sockaddr*)&newSockAddr, &newSockAddrSize);
|
||||
if (client > 0) {
|
||||
connectCallback(new ClientTCP(client));
|
||||
this->totalClientCounter++;
|
||||
}
|
||||
else {
|
||||
std::cerr << "Error accepting request from client!" << std::endl;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void ListenerTCP::ListenSyncAsync() {
|
||||
while (running && totalClientCounter < totalClientLimit) {
|
||||
sockaddr_in newSockAddr;
|
||||
socklen_t newSockAddrSize = sizeof(newSockAddr);
|
||||
int client = accept(s, (sockaddr*)&newSockAddr, &newSockAddrSize);
|
||||
if (client > 0) {
|
||||
ThreadPool::Enqueue([this, client]() {connectCallback(new ClientTCP(client)); });
|
||||
this->totalClientCounter++;
|
||||
}
|
||||
else {
|
||||
std::cerr << "Error accepting request from client!" << std::endl;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void ListenerTCP::ListenAsyncSync() {
|
||||
ThreadPool::Enqueue([this]() {
|
||||
while (running && totalClientCounter < totalClientLimit) {
|
||||
sockaddr_in newSockAddr;
|
||||
socklen_t newSockAddrSize = sizeof(newSockAddr);
|
||||
int client = accept(s, (sockaddr*)&newSockAddr, &newSockAddrSize);
|
||||
if (client > 0) {
|
||||
connectCallback(new ClientTCP(client));
|
||||
this->totalClientCounter++;
|
||||
}
|
||||
else {
|
||||
std::cerr << "Error accepting request from client!" << std::endl;
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
void ListenerTCP::ListenAsyncAsync() {
|
||||
ThreadPool::Enqueue([this]() {
|
||||
while (running && totalClientCounter < totalClientLimit) {
|
||||
sockaddr_in newSockAddr;
|
||||
socklen_t newSockAddrSize = sizeof(newSockAddr);
|
||||
int client = accept(s, (sockaddr*)&newSockAddr, &newSockAddrSize);
|
||||
if (client > 0) {
|
||||
ThreadPool::Enqueue([this, client]() {connectCallback(new ClientTCP(client)); });
|
||||
this->totalClientCounter++;
|
||||
}
|
||||
else {
|
||||
std::cerr << "Error accepting request from client!" << std::endl;
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
ListenerTCP::~ListenerTCP() {
|
||||
if(s != -1) {
|
||||
close(s);
|
||||
}
|
||||
//SPDX-License-Identifier: LGPL-3.0-only
|
||||
//SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||
|
||||
module;
|
||||
|
||||
#include <stdio.h>
|
||||
#include <sys/types.h>
|
||||
#include <sys/socket.h>
|
||||
#include <netinet/in.h>
|
||||
#include <arpa/inet.h>
|
||||
#include <sys/uio.h>
|
||||
#include <sys/time.h>
|
||||
#include <sys/wait.h>
|
||||
#include <strings.h>
|
||||
#include <cerrno>
|
||||
|
||||
module Crafter.Network:ListenerTCP_impl;
|
||||
import :ListenerTCP;
|
||||
import std;
|
||||
import Crafter.Thread;
|
||||
|
||||
using namespace Crafter;
|
||||
|
||||
ListenerTCP::ListenerTCP(std::uint16_t port, std::function<void(ClientTCP*)> connectCallback, std::uint32_t concurrentClientLimit, std::uint32_t totalClientLimit) : connectCallback(connectCallback), concurrentClientLimit(concurrentClientLimit), totalClientLimit(totalClientLimit) {
|
||||
sockaddr_in servAddr;
|
||||
bzero((char*)&servAddr, sizeof(servAddr));
|
||||
servAddr.sin_family = AF_INET;
|
||||
servAddr.sin_addr.s_addr = htonl(INADDR_ANY);
|
||||
servAddr.sin_port = htons(port);
|
||||
|
||||
s = socket(AF_INET, SOCK_STREAM, 0);
|
||||
if(s < 0)
|
||||
{
|
||||
throw std::runtime_error("Error establishing the server socket");
|
||||
}
|
||||
// Without SO_REUSEADDR the port stays unbindable for the length of
|
||||
// TIME_WAIT after a restart, which turns "restart the server" into a
|
||||
// minute of failed binds.
|
||||
int reuse = 1;
|
||||
setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &reuse, sizeof(reuse));
|
||||
|
||||
if(bind(s, (struct sockaddr*) &servAddr, sizeof(servAddr)) < 0) {
|
||||
// Ignoring this left the listener silently accepting nothing at
|
||||
// all, with no hint as to why.
|
||||
const std::string reason = std::strerror(errno);
|
||||
close(s);
|
||||
s = -1;
|
||||
throw std::runtime_error("Could not bind port " + std::to_string(port) + ": " + reason);
|
||||
}
|
||||
if(listen(s, 128) < 0) {
|
||||
const std::string reason = std::strerror(errno);
|
||||
close(s);
|
||||
s = -1;
|
||||
throw std::runtime_error("Could not listen on port " + std::to_string(port) + ": " + reason);
|
||||
}
|
||||
}
|
||||
|
||||
void ListenerTCP::Stop() {
|
||||
running = false;
|
||||
shutdown(s, SHUT_RDWR);
|
||||
close(s);
|
||||
s = -1;
|
||||
}
|
||||
|
||||
void ListenerTCP::ListenSyncSync() {
|
||||
while (running && totalClientCounter < totalClientLimit) {
|
||||
sockaddr_in newSockAddr;
|
||||
socklen_t newSockAddrSize = sizeof(newSockAddr);
|
||||
int client = accept(s, (sockaddr*)&newSockAddr, &newSockAddrSize);
|
||||
if (client > 0) {
|
||||
connectCallback(new ClientTCP(client));
|
||||
this->totalClientCounter++;
|
||||
}
|
||||
else if (running) {
|
||||
// accept() also fails once on the way out, when Stop() closes
|
||||
// the listening socket — that one is not worth reporting.
|
||||
std::cerr << "Error accepting request from client!" << std::endl;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void ListenerTCP::ListenSyncAsync() {
|
||||
while (running && totalClientCounter < totalClientLimit) {
|
||||
sockaddr_in newSockAddr;
|
||||
socklen_t newSockAddrSize = sizeof(newSockAddr);
|
||||
int client = accept(s, (sockaddr*)&newSockAddr, &newSockAddrSize);
|
||||
if (client > 0) {
|
||||
ThreadPool::Enqueue([this, client]() {connectCallback(new ClientTCP(client)); });
|
||||
this->totalClientCounter++;
|
||||
}
|
||||
else if (running) {
|
||||
std::cerr << "Error accepting request from client!" << std::endl;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void ListenerTCP::ListenAsyncSync() {
|
||||
ThreadPool::Enqueue([this]() {
|
||||
while (running && totalClientCounter < totalClientLimit) {
|
||||
sockaddr_in newSockAddr;
|
||||
socklen_t newSockAddrSize = sizeof(newSockAddr);
|
||||
int client = accept(s, (sockaddr*)&newSockAddr, &newSockAddrSize);
|
||||
if (client > 0) {
|
||||
connectCallback(new ClientTCP(client));
|
||||
this->totalClientCounter++;
|
||||
}
|
||||
else if (running) {
|
||||
std::cerr << "Error accepting request from client!" << std::endl;
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
void ListenerTCP::ListenAsyncAsync() {
|
||||
ThreadPool::Enqueue([this]() {
|
||||
while (running && totalClientCounter < totalClientLimit) {
|
||||
sockaddr_in newSockAddr;
|
||||
socklen_t newSockAddrSize = sizeof(newSockAddr);
|
||||
int client = accept(s, (sockaddr*)&newSockAddr, &newSockAddrSize);
|
||||
if (client > 0) {
|
||||
ThreadPool::Enqueue([this, client]() {connectCallback(new ClientTCP(client)); });
|
||||
this->totalClientCounter++;
|
||||
}
|
||||
else if (running) {
|
||||
std::cerr << "Error accepting request from client!" << std::endl;
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
ListenerTCP::~ListenerTCP() {
|
||||
if(s != -1) {
|
||||
close(s);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue