Skip to main content
Code Review

Return to Question

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link
Source Link
arcomber
  • 2.5k
  • 7
  • 28
  • 46

TCP client library using Winsock WSAEventSelect in C++ - Take 2

This is my second take on creating a tcp client C++ class. The class declaration is general but the implementation is Windows only at this stage and uses the Microsoft Winsock WSAEventSelect model.

The first attempt is here:

TCP client library using Winsock WSAEventSelect in C++

for some background.

The changes are mostly removing the static socket and event handle variables which meant a caller could only use one instance of the class in a program. Also the lifecycle of the thread used to handle socket events was flawed so that has been addressed.

I may have missed some test cases so would appreciate any comments whatsoever.

A concern I have is that events are raised and the library calls into user defined callback functions. These callback functions will be called from a different thread to the thread used to call the library functions so that could be a concern. not sure yet how to fix that issue.

Also there is no logging in the library itself - not really sure how to fix that one either. Possibly take an output stream for logging in the constructor.

Anyway, any comments at all would be very much appreciated.

tcpclient.hpp header:

/*
Description: Asynchronous socket client interface
Author: Angus Comber
*/
#ifndef TCPCLIENT_HPP_
#define TCPCLIENT_HPP_
#include <thread> // comms thread
#include <functional> // callback functions
namespace itl {
 enum SOCKET_ERROR_CODE { ERR_FATAL, ERR_INFORMATIONAL };
 // forward declare pimpl
 struct socketimpl;
 class tcpclient {
 public:
 /// construct with async connection, data received and exception callbacks 
 /// and optional data buffersize
 tcpclient(std::function<void()> connectcb, 
 std::function<void(const unsigned char*, unsigned)> receivecb, 
 std::function<void(const int, const char*)> exceptioncb, 
 const int buffersize = 4096);
 ~tcpclient();
 //! connect to a host
 bool connect(const char* host, unsigned short port);
 //! send byte stream to remote endpoint
 unsigned send(const unsigned char* data, unsigned length);
 //! send string remote endpoint
 unsigned send(const char* data);
 //! close connection
 int close();
 //! check if connected
 bool is_connected() const;
 tcpclient(const tcpclient&) = delete;
 tcpclient& operator=(const tcpclient&) = delete;
 private:
 int closesocket();
 int shutdown();
 void exception_event(const int error, const char* message);
 //! async connected event
 std::function<void()> connectfunc_;
 //! async data received event
 std::function<void(const unsigned char*, unsigned)> receivefunc_;
 //! async exception event
 std::function<void(const int, const char*)> exceptionfunc_;
 std::thread reader_thread;
 void comms_channel_handler();
 void stop_comms_channel();
 void start_comms_channel();
 bool shutting_down_;
 bool connected_;
 const int buffersize_;
 socketimpl *pimpl_; // Handle object - pimpl
 };
} // namespace itl
#endif // TCPCLIENT_HPP_

tcpclient implementation:

/*
Description: Implementation of asynchronous socket client 
using winsock event model
Author: Angus Comber
*/
#include "tcpclient.hpp"
#ifdef WIN32
#ifndef WIN32_LEAN_AND_MEAN
#define WIN32_LEAN_AND_MEAN
#endif
#include <winsock2.h> // Windows sockets v2
#include <ws2tcpip.h> // getaddrinfo
#pragma comment(lib,"ws2_32.lib") //winsock2 lib
#include <string>
namespace itl {
// hide implementation details from header
struct socketimpl
{
public:
 SOCKET socket_ = INVALID_SOCKET;
 WSAEVENT comms_event;
};
tcpclient::tcpclient(std::function<void()> connectcb, 
 std::function<void(const unsigned char*, unsigned)> receivecb, 
 std::function<void(const int, const char*)> exceptioncb, 
 const int buffersize)
 : connectfunc_(connectcb), receivefunc_(receivecb), exceptionfunc_(exceptioncb), 
 buffersize_(buffersize), shutting_down_(false), connected_(false),
 pimpl_(new socketimpl) {
 WSADATA w = { 0 };
 int error = WSAStartup(0x0202, &w);
 if (error || w.wVersion != 0x0202)
 { // there was an error
 throw "Could not initialise Winsock2";
 }
}
// creates socket and runs thread for processing socket comms
void tcpclient::start_comms_channel() {
 stop_comms_channel();
 pimpl_->socket_ = ::socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); // Create socket
 shutting_down_ = false;
 // launch listener thread to handle any received data
 reader_thread = std::thread(&tcpclient::comms_channel_handler, this);
 // yield main thread to give reader_thread time to get going
 std::this_thread::yield();
}
tcpclient::~tcpclient() {
 close();
 WSACleanup();
 delete pimpl_;
}
bool tcpclient::connect(const char* host, unsigned short port) {
 if (connected_) {
 exception_event(ERR_INFORMATIONAL, "attempt to connect failed - socket client already connected, disconnect first");
 return false; // already connected, client must disconnect first
 }
 addrinfo* result = NULL;
 addrinfo hints = {};
 sockaddr_in target = {};
 hints.ai_family = AF_UNSPEC;
 hints.ai_socktype = SOCK_STREAM;
 hints.ai_protocol = IPPROTO_TCP;
 // inet_pton() returns 1 on success. It returns -1 if there was an error (errno is set), 
 // or 0 if the input isn't a valid IP address.
 int ret = inet_pton(AF_INET, host, &(target.sin_addr));
 if (ret != 1) {
 // ok so we assume not a proper ip address, so try getaddrinfo - might be a hostname/domain name
 ret = getaddrinfo(host, NULL, &hints, &result);
 if (ret != 0) {
 std::string s = "getaddrinfo failed with error: " + std::to_string(ret) + '\n';
 exception_event(ERR_INFORMATIONAL, s.c_str());
 return false;
 }
 memcpy(&target, result->ai_addr, sizeof(sockaddr_in));
 }
 target.sin_family = AF_INET; // IPv4
 target.sin_port = htons(port);
 // stop comms thread created from a (possible) previous connection
 stop_comms_channel();
 
 // start reading thread now that we know connection arguments are valid
 start_comms_channel();
 ret = ::connect(pimpl_->socket_, reinterpret_cast<sockaddr *>(&target), sizeof(sockaddr));
 if (ret != 0) {
 ret = WSAGetLastError();
 if (ret == WSAEWOULDBLOCK) {
 // normal asynchronous connection
 ret = 0; 
 }
 else {
 char* s = NULL;
 FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
 NULL, ret,
 MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
 s, 0, NULL);
 exception_event(ERR_INFORMATIONAL, s);
 }
 }
 return ret == 0;
}
bool tcpclient::is_connected() const {
 return connected_;
}
unsigned tcpclient::send(const unsigned char* data, unsigned length) {
 return ::send(pimpl_->socket_, reinterpret_cast<const char*>(data), length, 0);
}
unsigned tcpclient::send(const char* data) {
 return ::send(pimpl_->socket_, data, strlen(data), 0);
}
int tcpclient::close() {
 stop_comms_channel();
 int ret = 0;
 if (pimpl_->socket_ != INVALID_SOCKET) {
 shutdown();
 ret = closesocket();
 }
 return ret;
}
int tcpclient::closesocket() {
 int ret = INVALID_SOCKET;
 if (pimpl_->socket_ != INVALID_SOCKET) {
 ret = ::closesocket(pimpl_->socket_);
 pimpl_->socket_ = INVALID_SOCKET;
 }
 return ret;
}
int tcpclient::shutdown() {
 int ret = INVALID_SOCKET;
 if (pimpl_->socket_ != INVALID_SOCKET) {
 // SD_SEND says to server, we have no more data to send
 // server may respond with response data
 ret = ::shutdown(pimpl_->socket_, SD_SEND);
 }
 return ret;
}
void tcpclient::exception_event(const int error, const char* message) {
 if (exceptionfunc_) {
 exceptionfunc_(error, message);
 }
 // cleanup if get an exception
 stop_comms_channel();
}
void tcpclient::stop_comms_channel() {
 // these 2 make thread loop stop - so thread runs out
 shutting_down_ = true;
 if (pimpl_->comms_event != WSA_INVALID_EVENT) {
 WSASetEvent(pimpl_->comms_event);
 }
 // you cannot join reader thread to itself - so check
 // wait for thread to stop
 if (reader_thread.joinable() && std::this_thread::get_id() != reader_thread.get_id()) {
 reader_thread.join(); // prevents crash - due to terminate being called on running thread still 'alive'
 }
}
void tcpclient::comms_channel_handler() {
 // Create an event object to be used with this socket
 pimpl_->comms_event = WSACreateEvent();
 if (pimpl_->comms_event == WSA_INVALID_EVENT)
 {
 exception_event(ERR_FATAL, "Error creating winsock WSACreateEvent object");
 }
 // setup winsock event handling 
 int ret = WSAEventSelect(pimpl_->socket_, pimpl_->comms_event, FD_READ | FD_CONNECT | FD_CLOSE);
 if (ret != 0) {
 int lasterror = WSAGetLastError();
 std::string s = "Winsock communication error - unable to create event object winsock error " + std::to_string(lasterror);
 exception_event(ERR_FATAL, s.c_str()); 
 return;
 }
 // Handle async network events
 WSANETWORKEVENTS events;
 char* buffer = new char[buffersize_]();
 while (!shutting_down_) {
 // Wait for a socket event
 DWORD dwRet = WSAWaitForMultipleEvents(1, &pimpl_->comms_event, FALSE, WSA_INFINITE, FALSE);
 if (dwRet == WSA_WAIT_TIMEOUT)
 {
 // will never get to here as using WSA_INFINITE
 break;
 }
 // Type of event that occurred on socket_
 int nRet = WSAEnumNetworkEvents(pimpl_->socket_, pimpl_->comms_event, &events);
 if (nRet == SOCKET_ERROR)
 {
 exception_event(ERR_FATAL, "Winsock WaitForCommsEvent socket error");
 break;
 }
 // Handle events
 // Connect event
 if (events.lNetworkEvents & FD_CONNECT)
 {
 if (events.iErrorCode[FD_CONNECT_BIT] != 0)
 {
 
 // if asynchronous connect failure, we get informed here
 exception_event(ERR_FATAL, "Winsock events error code for FD_CONNECT_BIT");
 break;
 }
 connected_ = true;
 if (connectfunc_) {
 connectfunc_();
 }
 }
 // Read event
 if (events.lNetworkEvents & FD_READ)
 {
 // No need to keep reading, will get a new event for any data not read
 int bytes = recv(pimpl_->socket_, buffer, buffersize_, 0);
 if (bytes == SOCKET_ERROR)
 {
 exception_event(ERR_FATAL, "Winsock socket error reading received data");
 break;
 }
 if (bytes > 0)
 {
 if (receivefunc_) {
 receivefunc_(reinterpret_cast<const unsigned char*>(buffer), bytes);
 }
 }
 }
 // Close event
 if (events.lNetworkEvents & FD_CLOSE)
 {
 connected_ = false;
 exception_event(ERR_INFORMATIONAL, "Socket closed");
 break;
 }
 }
 pimpl_->comms_event = WSA_INVALID_EVENT;
 delete [] buffer;
}
} // namespace itl
#elif
#error Only Windows platform supported
#endif

main.cpp testing/exercising source:

#include "tcpclient.hpp"
#include <stdio.h>
#include <string.h>
#include <chrono>
#include <iostream>
static void printhex(const unsigned char* data, const unsigned len)
{
 for (size_t i = 0; i < len; ++i) {
 printf("%02x ", data[i]);
 if ((i + 1) % 16 == 0)
 putchar('\n');
 }
 putchar('\n');
}
class http_client
{
public:
 http_client() : client(NULL) {
 client = new itl::tcpclient([this]() { connect_handler(); }, 
 [this](const unsigned char* data, unsigned length) { data_handler(data, length); }, 
 [this](const int errorcode, const char* error_message) { exception_handler(errorcode, error_message); });
 
 printf("http_client ctor called\n");
 }
 ~http_client() { delete client; printf("http_client dtor called\n"); }
 void close() { client->close(); printf("http_client close called\n"); }
 void connect(const char* host, const unsigned short port) {
 printf("http_client connect called\n");
 host_ = host;
 port_ = port;
 client->connect(host, port);
 }
 void do_connect() {
 printf("do_connect called\n");
 std::string url = "GET / HTTP/1.0\r\nHost: " + host_ + "\r\nConnection: keep-alive\r\nUser-Agent: test socket program/1.0\r\nAccept-Encoding: gzip/\r\nAccept-Charset: ISO-8859-1,UTF-8;q=0.7,*;q=0.7\r\nCache-Control: no - cache/\r\nAccept-Language: de, en; q = 0.7, en - us; q = 0.3/\r\n\r\n";
 client->send(url.c_str());
 }
 void data_handler(const unsigned char* data, unsigned length) {
 printf("data_handler called %u bytes\n", length);
 printhex(data, length);
 }
 void connect_handler() {
 printf("connect_handler called\n");
 do_connect();
 }
 void exception_handler(const int errorcode, const char* error_message) {
 printf("Error %d, %s\n", errorcode, error_message);
 }
private:
 std::string host_;
 unsigned port_;
 itl::tcpclient* client;
 http_client(const http_client&) = delete;
 http_client& operator=(const http_client&) = delete;
};
int main() {
 http_client client; 
 // bogus host
 client.connect("codereview.stockexchange.com", 80);
 // stop program finishing before we have downloaded all web content
 for (int i = 0; i < 5; ++i) {
 putchar('.');
 std::this_thread::sleep_for(std::chrono::seconds(1));
 }
 // this host does connect - but immediate rejection
 client.connect("codereview.stickexchange.com", 80);
 // stop program finishing before we have downloaded all web content
 for (int i = 0; i < 5; ++i) {
 putchar('.');
 std::this_thread::sleep_for(std::chrono::seconds(1));
 }
 // now try successful case - these hosts set Connection Keep-alive meaning they don't immediately
 // close the connection and so we close at end of test
 http_client client2; // test for concurrent clients
 http_client client3;
 client.connect("codereview.stackexchange.com", 80);
 client2.connect("codereview.stackexchange.com", 80);
 client3.connect("codereview.stackexchange.com", 80);
 // stop program finishing before we have downloaded all web content
 for (int i = 0; i < 5; ++i) {
 putchar('.');
 std::this_thread::sleep_for(std::chrono::seconds(1));
 }
 client.close();
 client2.close();
 client3.close();
}
lang-cpp

AltStyle によって変換されたページ (->オリジナル) /