From 043929d85037306e5139daa91a1a638d7f444dcf Mon Sep 17 00:00:00 2001 From: Klaus Schmidinger Date: Sat, 17 Mar 2018 10:07:19 +0100 Subject: Fixed a possible race condition with CONN vs. LSTT --- svdrp.c | 40 ++++++++++++++++++++++++---------------- svdrp.h | 12 ++---------- timers.c | 16 +++++++++++++--- 3 files changed, 39 insertions(+), 29 deletions(-) diff --git a/svdrp.c b/svdrp.c index 32f9f882..043ffafc 100644 --- a/svdrp.c +++ b/svdrp.c @@ -10,7 +10,7 @@ * and interact with the Video Disk Recorder - or write a full featured * graphical interface that sits on top of an SVDRP connection. * - * $Id: svdrp.c 4.33 2018/03/04 14:15:07 kls Exp $ + * $Id: svdrp.c 4.34 2018/03/17 10:06:54 kls Exp $ */ #include "svdrp.h" @@ -47,6 +47,13 @@ static bool DumpSVDRPDataTransfer = false; static int SVDRPTcpPort = 0; static int SVDRPUdpPort = 0; +enum eSvdrpFetchFlags { + sffNone = 0b00000000, + sffConn = 0b00000001, + sffPing = 0b00000010, + sffTimers = 0b00000100, + }; + // --- cIpAddress ------------------------------------------------------------ class cIpAddress { @@ -327,8 +334,8 @@ public: bool Process(cStringList *Response = NULL); bool Execute(const char *Command, cStringList *Response = NULL); bool Connected(void) const { return connected; } - void SetFetchFlag(eSvdrpFetchFlags Flag); - bool HasFetchFlag(eSvdrpFetchFlags Flag); + void SetFetchFlag(int Flag); + bool HasFetchFlag(int Flag); bool GetRemoteTimers(cStringList &Response); }; @@ -415,8 +422,7 @@ bool cSVDRPClient::Process(cStringList *Response) serverName = n; dsyslog("SVDRP %s < %s remote server name is '%s'", Setup.SVDRPHostName, serverIpAddress.Connection(), *serverName); } - Execute(cString::sprintf("CONN name:%s port:%d vdrversion:%d apiversion:%d timeout:%d", Setup.SVDRPHostName, SVDRPTcpPort, VDRVERSNUM, APIVERSNUM, Setup.SVDRPTimeout)); - SetFetchFlag(sffTimers); + SetFetchFlag(sffConn | sffTimers); connected = true; } } @@ -456,7 +462,7 @@ bool cSVDRPClient::Process(cStringList *Response) break; // we read all or nothing! } if (pingTime.TimedOut()) - Execute("PING"); + SetFetchFlag(sffPing); } return file.IsOpen(); } @@ -468,12 +474,12 @@ bool cSVDRPClient::Execute(const char *Command, cStringList *Response) return Send(Command) && Process(Response); } -void cSVDRPClient::SetFetchFlag(eSvdrpFetchFlags Flags) +void cSVDRPClient::SetFetchFlag(int Flags) { fetchFlags |= Flags; } -bool cSVDRPClient::HasFetchFlag(eSvdrpFetchFlags Flag) +bool cSVDRPClient::HasFetchFlag(int Flag) { bool Result = (fetchFlags & Flag); fetchFlags &= ~Flag; @@ -588,7 +594,7 @@ public: void Unlock(void) { mutex.Unlock(); } void AddClient(cSVDRPServerParams &ServerParams, const char *IpAddress); bool Execute(const char *ServerName, const char *Command, cStringList *Response = NULL); - bool GetServerNames(cStringList *ServerNames, eSvdrpFetchFlags FetchFlags = sffNone); + bool GetServerNames(cStringList *ServerNames); bool TriggerFetchingTimers(const char *ServerName); }; @@ -635,6 +641,10 @@ void cSVDRPClientHandler::ProcessConnections(void) for (int i = 0; i < clientConnections.Size(); i++) { cSVDRPClient *Client = clientConnections[i]; if (Client->Process()) { + if (Client->HasFetchFlag(sffConn)) + Client->Execute(cString::sprintf("CONN name:%s port:%d vdrversion:%d apiversion:%d timeout:%d", Setup.SVDRPHostName, SVDRPTcpPort, VDRVERSNUM, APIVERSNUM, Setup.SVDRPTimeout)); + if (Client->HasFetchFlag(sffPing)) + Client->Execute("PING"); if (Client->HasFetchFlag(sffTimers)) { cStringList RemoteTimers; if (Client->GetRemoteTimers(RemoteTimers)) { @@ -712,16 +722,14 @@ bool cSVDRPClientHandler::Execute(const char *ServerName, const char *Command, c return false; } -bool cSVDRPClientHandler::GetServerNames(cStringList *ServerNames, eSvdrpFetchFlags FetchFlag) +bool cSVDRPClientHandler::GetServerNames(cStringList *ServerNames) { cMutexLock MutexLock(&mutex); ServerNames->Clear(); for (int i = 0; i < clientConnections.Size(); i++) { cSVDRPClient *Client = clientConnections[i]; - if (Client->Connected()) { - if (FetchFlag == sffNone || Client->HasFetchFlag(FetchFlag)) - ServerNames->Append(strdup(Client->ServerName())); - } + if (Client->Connected()) + ServerNames->Append(strdup(Client->ServerName())); } return ServerNames->Size() > 0; } @@ -2744,13 +2752,13 @@ void StopSVDRPHandler(void) SVDRPServerHandler = NULL; } -bool GetSVDRPServerNames(cStringList *ServerNames, eSvdrpFetchFlags FetchFlag) +bool GetSVDRPServerNames(cStringList *ServerNames) { bool Result = false; cMutexLock MutexLock(&SVDRPHandlerMutex); if (SVDRPClientHandler) { SVDRPClientHandler->Lock(); - Result = SVDRPClientHandler->GetServerNames(ServerNames, FetchFlag); + Result = SVDRPClientHandler->GetServerNames(ServerNames); SVDRPClientHandler->Unlock(); } return Result; diff --git a/svdrp.h b/svdrp.h index 167209c4..5330b9aa 100644 --- a/svdrp.h +++ b/svdrp.h @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: svdrp.h 4.10 2018/03/04 12:26:17 kls Exp $ + * $Id: svdrp.h 4.11 2018/03/15 16:30:29 kls Exp $ */ #ifndef __SVDRP_H @@ -19,11 +19,6 @@ enum eSvdrpPeerModes { spmOnly = 2, }; -enum eSvdrpFetchFlags { - sffNone = 0b0000, - sffTimers = 0b0001, - }; - extern cStateKey StateKeySVDRPRemoteTimersPoll; ///< Controls whether a change to the local list of timers needs to result in ///< sending a POLL to the remote clients. @@ -32,13 +27,10 @@ void SetSVDRPPorts(int TcpPort, int UdpPort); void SetSVDRPGrabImageDir(const char *GrabImageDir); void StartSVDRPHandler(void); void StopSVDRPHandler(void); -bool GetSVDRPServerNames(cStringList *ServerNames, eSvdrpFetchFlags FetchFlag = sffNone); +bool GetSVDRPServerNames(cStringList *ServerNames); ///< Gets a list of all available VDRs this VDR is connected to via SVDRP, ///< and stores it in the given ServerNames list. The list is cleared ///< before getting the server names. - ///< If FetchFlag is given, only the server names for which the local - ///< client has this flag set will be returned, and the client's flag - ///< will be cleared. ///< Returns true if the resulting list is not empty. bool ExecSVDRPCommand(const char *ServerName, const char *Command, cStringList *Response = NULL); ///< Sends the given SVDRP Command string to the remote VDR identified diff --git a/timers.c b/timers.c index 910ddb7a..8119b858 100644 --- a/timers.c +++ b/timers.c @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: timers.c 4.17 2018/03/11 13:03:23 kls Exp $ + * $Id: timers.c 4.18 2018/03/17 10:07:19 kls Exp $ */ #include "timers.h" @@ -952,8 +952,14 @@ bool cTimers::StoreRemoteTimers(const char *ServerName, const cStringList *Remot else // processed all right entries DelTimer = nl; } - else if (ir < sr) // still have right entries + else if (ir < sr) { // still have right entries AddTimer = atoi((*RemoteTimers)[ir]); + if (!AddTimer) { + esyslog("ERROR: %s: error in timer settings: %s", ServerName, (*RemoteTimers)[ir]); + ir++; + continue; // let's see if we can process the rest + } + } else // processed all left and right entries break; if (AddTimer && DelTimer) { @@ -971,7 +977,7 @@ bool cTimers::StoreRemoteTimers(const char *ServerName, const cStringList *Remot Result = true; } else - esyslog("ERROR: %s: error in timer settings: %s", ServerName, v); + esyslog("ERROR: %d@%s: error in timer settings: %s", DelTimer, ServerName, v); } } else // identical timer, nothing to do @@ -1003,6 +1009,10 @@ bool cTimers::StoreRemoteTimers(const char *ServerName, const cStringList *Remot } il++; } + else { + esyslog("ERROR: oops while storing remote timers!"); + break; // let's not get stuck here! + } } return Result; } -- cgit v1.2.3