From 44163a536e68231f8ba248a2c444e0f0035f9004 Mon Sep 17 00:00:00 2001 From: Chris Tallon Date: Thu, 23 Apr 2020 16:14:12 +0100 Subject: [PATCH] VDR connection bug fixes - Added util.h util.cc - Bug fix: Reconnecting after connection lost - Bug fix: kill -TERM handling fixed during VDP - Bug fix: kill -TERM during VDR connecting now works --- boxstack.cc | 6 ++-- tcp.cc | 39 ++++++++++++++++++----- tcp.h | 2 +- util.cc | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++ util.h | 35 +++++++++++++++++++++ vconnect.cc | 8 ++++- vdr.cc | 85 +++++++++++++++++++++++++++++++++++-------------- vdr.h | 8 ++++- 8 files changed, 236 insertions(+), 38 deletions(-) create mode 100644 util.cc create mode 100644 util.h diff --git a/boxstack.cc b/boxstack.cc index d72500b..eaa0110 100644 --- a/boxstack.cc +++ b/boxstack.cc @@ -489,6 +489,7 @@ void BoxStack::removeAllExceptWallpaper() toDel = NULL; } + // FIXME what is this? if (!videoStack.empty() && videoStack.top().first==toDel) { videoStack.pop(); if (!videoStack.empty()) display=&videoStack.top().second; @@ -499,11 +500,8 @@ void BoxStack::removeAllExceptWallpaper() Log::getInstance()->log("BoxStack", Log::DEBUG, "going to delete boxx %p, num=%d", toDel, numBoxes); if (display) Video::getInstance()->setVideoDisplay(*display); - if (toDel) delete toDel; } - - } int BoxStack::handleCommand(int command) @@ -522,7 +520,7 @@ int BoxStack::handleCommand(int command) for (i=numBoxes-1; i>=0; i--) { -// Log::getInstance()->log("BoxStack", Log::DEBUG, "Giving command to i=%i", i); + // Log::getInstance()->log("BoxStack", Log::DEBUG, "Giving command to i=%i", i); retVal = boxes[i]->handleCommand(command); if (retVal == 1) { diff --git a/tcp.cc b/tcp.cc index 0f2f248..96d315c 100644 --- a/tcp.cc +++ b/tcp.cc @@ -137,27 +137,42 @@ bool TCP::connect(const std::string& ip, USHORT port) // Wait for connect to complete fd_set writefds; - struct timeval tv; FD_ZERO(&writefds); FD_SET(sockfd, &writefds); int maxfd = sockfd; + fd_set readfds; + FD_ZERO(&readfds); + + #ifdef WIN32 - FD_SET(abortSocket, &writefds); + FD_SET(abortSocket, &readfds); #else - FD_SET(abortPipe[0], &writefds); + FD_SET(abortPipe[0], &readfds); if (abortPipe[0] > maxfd) maxfd = abortPipe[0]; #endif - tv.tv_sec = 5; // Allow 5s for a connect + struct timeval tv; + tv.tv_sec = 20; // Allow 5s for a connect tv.tv_usec = 0; - int selectResult = select(maxfd + 1, NULL, &writefds, NULL, &tv); + int selectResult = select(maxfd + 1, &readfds, &writefds, NULL, &tv); +#ifdef WIN32 + if (FD_ISSET(abortSocket, &readfds)) +#else + if (FD_ISSET(abortPipe[0], &readfds)) +#endif + { + CLOSESOCKET(sockfd); + sockfd = -1; + logger->log("TCP", Log::INFO, "connect/select aborting"); + return false; + } if ((selectResult == 1) || FD_ISSET(sockfd, &writefds)) { freeaddrinfo(aip); - logger->log("TCP", Log::INFO, "connected"); + logger->log("TCP", Log::INFO, "Connected"); connected = true; return true; } @@ -176,6 +191,13 @@ void TCP::shutdown() connected = false; CLOSESOCKET(sockfd); sockfd = -1; + + char waste[10]; +#ifdef WIN32 + while (recv(abortSocket, waste, 10, 0) > 0) ; +#else + while (::read(abortPipe[0], waste, 10) > 0) ; +#endif } bool TCP::status() @@ -245,7 +267,10 @@ bool TCP::read(void* dst, ULONG numBytes) #ifdef WIN32 if (FD_ISSET(abortSocket, &readfds)) { logger->log("TCP", Log::DEBUG, "aborting..."); return false; } #else - if (FD_ISSET(abortPipe[0], &readfds)) { logger->log("TCP", Log::DEBUG, "aborting..."); return false; } + + + + if (FD_ISSET(abortPipe[0], &readfds)) { logger->log("TCP", Log::DEBUG, "B aborting..."); return false; } #endif int recvResult = recv(sockfd, pointer, numBytes - totalReceived, 0); diff --git a/tcp.h b/tcp.h index 01034bf..39398f9 100644 --- a/tcp.h +++ b/tcp.h @@ -40,7 +40,7 @@ class TCP public: ~TCP(); bool init(); // Must call this first, once on any new TCP object - void shutdown(); // Optional. Closes connection. Can call connect() next + void shutdown(); // Optional if this is to be deleted. Closes connection. Can call connect() next void abortCall(); // causes a read/connect call to immediately abort bool connect(const std::string& ip, USHORT port); diff --git a/util.cc b/util.cc new file mode 100644 index 0000000..f46afdf --- /dev/null +++ b/util.cc @@ -0,0 +1,91 @@ +/* + Copyright 2020 Chris Tallon + + This file is part of VOMP. + + VOMP is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + VOMP is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with VOMP. If not, see . +*/ + +#include +#include + +#ifdef WIN32 +#include +#endif + +#include "util.h" + +void MILLISLEEP(ULONG a) +{ +#ifndef WIN32 + struct timespec delayTime; + delayTime.tv_sec = a / 1000; + delayTime.tv_nsec = (a % 1000) * 1000000; + nanosleep(&delayTime, NULL); +#else + Sleep(a); +#endif +} + +std::string tp2str(const std::chrono::time_point& tp) +{ + auto tms = std::chrono::time_point_cast(tp); + std::chrono::milliseconds e = tms.time_since_epoch(); + long long c = e.count(); + time_t tt = c / 1000; + int ttm = c % 1000; + auto stm = std::localtime(&tt); + std::stringstream ss; + ss << std::put_time(stm, "%T") << "." << std::setfill('0') << std::setw(3) << ttm; + return ss.str(); +} + +/* +ULLONG htonll(ULLONG a) +{ + return (((ULLONG)htonl((ULONG)((a<<32)>> 32))<<32) + |(ULONG)htonl(((ULONG) (a >> 32)))); +} + +ULLONG ntohll(ULLONG a) +{ + return htonll(a); +} + +ULLONG htonll(ULLONG a) +{ + #if BYTE_ORDER == BIG_ENDIAN + return a; + #else + ULLONG b = 0; + + b = ((a << 56) & 0xFF00000000000000ULL) + | ((a << 40) & 0x00FF000000000000ULL) + | ((a << 24) & 0x0000FF0000000000ULL) + | ((a << 8) & 0x000000FF00000000ULL) + | ((a >> 8) & 0x00000000FF000000ULL) + | ((a >> 24) & 0x0000000000FF0000ULL) + | ((a >> 40) & 0x000000000000FF00ULL) + | ((a >> 56) & 0x00000000000000FFULL) ; + + return b; + #endif +} + +ULLONG ntohll(ULLONG a) +{ + return htonll(a); +} + +*/ diff --git a/util.h b/util.h new file mode 100644 index 0000000..6701048 --- /dev/null +++ b/util.h @@ -0,0 +1,35 @@ +/* + Copyright 2020 Chris Tallon + + This file is part of VOMP. + + VOMP is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + VOMP is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with VOMP. If not, see . +*/ + +#ifndef UTIL_H +#define UTIL_H + +#include +#include + +#include "defines.h" + +void MILLISLEEP(ULONG a); +std::string tp2str(const std::chrono::time_point& tp); + + +//ULLONG htonll(ULLONG a); +//ULLONG ntohll(ULLONG a); + +#endif diff --git a/vconnect.cc b/vconnect.cc index b9fe4d8..c140dfe 100644 --- a/vconnect.cc +++ b/vconnect.cc @@ -88,6 +88,7 @@ void VConnect::stop() { threadMutex.lock(); threadReqQuit = true; + vdr->abortConnect(); // If this and vdr are connecting, cancel it threadCond.notify_one(); threadMutex.unlock(); connectThread.join(); @@ -158,7 +159,10 @@ void VConnect::threadMethod() draw(); boxstack->update(this); + if (threadReqQuit) return; success = vdr->connect(); + if (threadReqQuit) return; + if (success) { logger->log("VConnect", Log::DEBUG, "Connected ok, doing login"); @@ -188,7 +192,9 @@ void VConnect::threadMethod() draw(); boxstack->update(this); - MILLISLEEP(delay); + + MILLISLEEP(delay); // FIXME wait on cond? + if (threadReqQuit) return; } while(!success); logger->log("VConnect", Log::INFO, "Send VDR connected message"); diff --git a/vdr.cc b/vdr.cc index 632a6b8..e756814 100644 --- a/vdr.cc +++ b/vdr.cc @@ -161,31 +161,71 @@ void VDR::setServerPort(USHORT newPort) serverPort = newPort; } + int VDR::connect() { maxChannelNumber = 0; channelNumberWidth = 1; - tcp.shutdown(); - if (!tcp.connect(serverIP, serverPort)) return 0; + connectStateMutex.lock(); + if (connecting || connected) return 0; + connecting = true; + connectStateMutex.unlock(); // now I have connecting + + bool connectResult = tcp.connect(serverIP, serverPort); - connected = true; + connectStateMutex.lock(); // connected = false, connecting = true, babortConnect = ? + connecting = false; - threadStartProtect.lock(); - vdrThread = std::thread( [this] + if (babortConnect) { + if (connectResult) tcp.shutdown(); + babortConnect = false; + connectStateMutex.unlock(); + return 0; + } + + if (connectResult) + { + connected = true; + threadStartProtect.lock(); + vdrThread = std::thread( [this] + { + threadStartProtect.lock(); + threadStartProtect.unlock(); + threadMethod(); + }); threadStartProtect.unlock(); - threadMethod(); - }); - threadStartProtect.unlock(); - return 1; + connectStateMutex.unlock(); + return 1; + } + else + { + connectStateMutex.unlock(); + return 0; + } +} + +void VDR::abortConnect() // If there is one, force a running connect call to abort +{ + connectStateMutex.lock(); + + if (connecting) + { + babortConnect = true; + tcp.abortCall(); + } + + connectStateMutex.unlock(); } void VDR::disconnect() { - logger->log("VDR", Log::DEBUG, "Disconnect start"); + std::lock_guard lg(connectStateMutex); + + if (!connected) return; if (vdrThread.joinable()) { @@ -193,9 +233,11 @@ void VDR::disconnect() tcp.abortCall(); vdrThread.join(); threadReqStop = false; - logger->log("VDR", Log::DEBUG, "done thread stop"); } + tcp.shutdown(); + + disconnecting = false; connected = false; logger->log("VDR", Log::DEBUG, "Disconnect"); } @@ -229,21 +271,15 @@ void VDR::threadMethod() while(1) { - if (threadReqStop) return; - - timeNow = time(NULL); - readSuccess = tcp.read(&channelID, sizeof(ULONG)); - if (threadReqStop) return; + timeNow = time(NULL); if (!readSuccess) { //logger->log("VDR", Log::DEBUG, "Net read timeout"); if (!tcp.status()) { connectionDied(); return; } // return to stop this thread } - - // Error or timeout. if (!lastKAsent) // have not sent a KA { @@ -392,7 +428,7 @@ void VDR::connectionDied() { // Called from within threadMethod to do cleanup if it decides the connection has died - connected = false; // though actually it could still be connected until someone calls vdr->disconnect + disconnecting = true; // Need to wake up any waiting channel 1 request-response threads // Normally this is done by a packet coming in with channelid and requestid @@ -482,9 +518,9 @@ VDR_ResponsePacket* VDR::RequestResponse(VDR_RequestPacket* vrp) { //logger->log("VDR", Log::DEBUG, "RR %lu", vrp->getOpcode()); - if (!connected) + if (!connected || disconnecting) { - logger->log("VDR", Log::DEBUG, "RR when !connected"); + logger->log("VDR", Log::DEBUG, "RR when !connected || disconnecting"); VDR_ResponsePacket* vresp = new VDR_ResponsePacket(); return vresp; // "no-response" return } @@ -684,7 +720,7 @@ int VDR::doLogin(unsigned int* v_server_min, unsigned int* v_server_max, unsigne bool VDR::LogExtern(const char* logString) { - if (!connected) return false; + if (!connected || disconnecting) return false; int stringLength = strlen(logString); int packetLength = stringLength + 8; char *buffer=new char[packetLength + 1]; @@ -704,8 +740,9 @@ bool VDR::LogExtern(const char* logString) if (!tcp.write(buffer, packetLength)) { - connected = false; // stop the rest of the connection delete [] buffer; + disconnecting = true; // stop the rest of the connection + Control::getInstance()->connectionLost(); return false; } delete [] buffer; @@ -1580,7 +1617,7 @@ void VDR::shutdownVDR() else logger->log("VDR", Log::DEBUG, "Shutting down vomp only"); - if(!doVDRShutdown || !connected) + if(!doVDRShutdown || !connected || disconnecting) return; VDR_RequestPacket vrp; diff --git a/vdr.h b/vdr.h index ad00955..b7bd161 100644 --- a/vdr.h +++ b/vdr.h @@ -129,6 +129,7 @@ public ExternLogger void setReceiveWindow(size_t size); int connect(); void disconnect(); + void abortConnect(); // If there is one, force a running connect call to abort bool isConnected() { return connected; } int getChannelNumberWidth() { return channelNumberWidth; } @@ -234,12 +235,17 @@ public ExternLogger TCP tcp; char serverIP[40]; USHORT serverPort; - bool connected{}; ULONG maxChannelNumber{}; bool doVDRShutdown{}; int channelNumberWidth{1}; VDR_PacketReceiver* TEMP_SINGLE_VDR_PR; + std::mutex connectStateMutex; // FIXME improve this + bool connecting{}; + bool disconnecting{}; + bool babortConnect{}; + bool connected{}; + std::mutex threadStartProtect; std::thread vdrThread; bool threadReqStop{}; -- 2.39.5