From c76395abc60a132b6a815a279a1b5f46baa2670d Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Fri, 10 Jan 2025 12:39:47 -0800 Subject: [PATCH 01/15] remove fdmgrTest Not a unittest, and not functional. --- modules/libcom/test/Makefile | 5 -- modules/libcom/test/fdmgrTest.c | 139 -------------------------------- 2 files changed, 144 deletions(-) delete mode 100644 modules/libcom/test/fdmgrTest.c diff --git a/modules/libcom/test/Makefile b/modules/libcom/test/Makefile index a3893661f..99e7e2bd5 100644 --- a/modules/libcom/test/Makefile +++ b/modules/libcom/test/Makefile @@ -315,11 +315,6 @@ TESTPROD_HOST += buckTest buckTest_SRCS += buckTest.c testHarness_SRCS += buckTest.c -#TESTPROD_HOST += fdmgrTest -fdmgrTest_SRCS += fdmgrTest.c -fdmgrTest_LIBS += ca -# FIXME: program never exits. - TESTPROD_HOST += epicsAtomicPerform epicsAtomicPerform_SRCS += epicsAtomicPerform.cpp testHarness_SRCS += epicsAtomicPerform.cpp diff --git a/modules/libcom/test/fdmgrTest.c b/modules/libcom/test/fdmgrTest.c deleted file mode 100644 index a7f91d493..000000000 --- a/modules/libcom/test/fdmgrTest.c +++ /dev/null @@ -1,139 +0,0 @@ -/*************************************************************************\ -* Copyright (c) 2006 UChicago Argonne LLC, as Operator of Argonne -* National Laboratory. -* Copyright (c) 2002 The Regents of the University of California, as -* Operator of Los Alamos National Laboratory. -* SPDX-License-Identifier: EPICS -* EPICS BASE is distributed subject to a Software License Agreement found -* in file LICENSE that is included with this distribution. -\*************************************************************************/ - -#include -#include - -#include "fdmgr.h" -#include "epicsTime.h" -#include "epicsAssert.h" -#include "cadef.h" - -#define verify(exp) ((exp) ? (void)0 : \ - epicsAssert(__FILE__, __LINE__, #exp, epicsAssertAuthor)) - -static const unsigned uSecPerSec = 1000000; - -typedef struct cbStructCreateDestroyFD { - fdctx *pfdm; - int trig; -} cbStructCreateDestroyFD; - -void fdHandler (void *pArg) -{ - cbStructCreateDestroyFD *pCBFD = (cbStructCreateDestroyFD *) pArg; - - printf ("triggered\n"); - pCBFD->trig = 1; -} - -void fdCreateDestroyHandler (void *pArg, int fd, int open) -{ - cbStructCreateDestroyFD *pCBFD = (cbStructCreateDestroyFD *) pArg; - int status; - - if (open) { - printf ("new fd = %d\n", fd); - status = fdmgr_add_callback (pCBFD->pfdm, fd, fdi_read, fdHandler, pArg); - verify (status==0); - } - else { - printf ("terminated fd = %d\n", fd); - status = fdmgr_clear_callback (pCBFD->pfdm, fd, fdi_read); - verify (status==0); - } -} - -typedef struct cbStuctTimer { - epicsTimeStamp time; - int done; -} cbStruct; - -void alarmCB (void *parg) -{ - cbStruct *pCBS = (cbStruct *) parg; - epicsTimeGetCurrent (&pCBS->time); - pCBS->done = 1; -} - -void testTimer (fdctx *pfdm, double delay) -{ - int status; - fdmgrAlarmId aid; - struct timeval tmo; - epicsTimeStamp begin; - cbStruct cbs; - double measuredDelay; - double measuredError; - - epicsTimeGetCurrent (&begin); - cbs.done = 0; - tmo.tv_sec = (time_t) delay; - tmo.tv_usec = (unsigned long) ((delay - tmo.tv_sec) * uSecPerSec); - aid = fdmgr_add_timeout (pfdm, &tmo, alarmCB, &cbs); - verify (aid!=fdmgrNoAlarm); - - while (!cbs.done) { - tmo.tv_sec = (time_t) delay; - tmo.tv_usec = (unsigned long) ((delay - tmo.tv_sec) * uSecPerSec); - status = fdmgr_pend_event (pfdm, &tmo); - verify (status==0); - } - - measuredDelay = epicsTimeDiffInSeconds (&cbs.time, &begin); - measuredError = fabs (measuredDelay-delay); - printf ("measured delay for %lf sec was off by %lf sec (%lf %%)\n", - delay, measuredError, 100.0*measuredError/delay); -} - -int main (int argc, char **argv) -{ - int status; - fdctx *pfdm; - cbStructCreateDestroyFD cbsfd; - struct timeval tmo; - chid chan; - - pfdm = fdmgr_init (); - verify (pfdm); - - SEVCHK (ca_task_initialize(), NULL); - cbsfd.pfdm = pfdm; - SEVCHK (ca_add_fd_registration (fdCreateDestroyHandler, &cbsfd), NULL); - - /* - * timer test - */ - testTimer (pfdm, 0.001); - testTimer (pfdm, 0.01); - testTimer (pfdm, 0.1); - testTimer (pfdm, 1.0); - - if (argc==2) { - SEVCHK(ca_search (argv[1], &chan), NULL); - } - - while (1) { - tmo.tv_sec = 0; - tmo.tv_usec = 100000; - cbsfd.trig = 0; - status = fdmgr_pend_event (pfdm, &tmo); - verify (status==0); - ca_poll (); - } - - status = fdmgr_delete (pfdm); - verify (status==0); - - printf ( "Test Complete\n" ); - - return 0; -} - From 8f77e941c76f858d49331127ddfe576bbbf73d54 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Fri, 10 Jan 2025 15:56:05 -0800 Subject: [PATCH 02/15] add fdManager test --- modules/libcom/test/Makefile | 5 + modules/libcom/test/epicsRunLibComTests.c | 2 + modules/libcom/test/fdManagerTest.cpp | 312 ++++++++++++++++++++++ 3 files changed, 319 insertions(+) create mode 100644 modules/libcom/test/fdManagerTest.cpp diff --git a/modules/libcom/test/Makefile b/modules/libcom/test/Makefile index 99e7e2bd5..6e5bd19a7 100644 --- a/modules/libcom/test/Makefile +++ b/modules/libcom/test/Makefile @@ -209,6 +209,11 @@ aslibtest_SRCS += aslibtest.c testHarness_SRCS += aslibtest.c TESTS += aslibtest +TESTPROD_HOST += fdManagerTest +fdManagerTest_SRCS += fdManagerTest.cpp +testHarness_SRCS += fdManagerTest.cpp +TESTS += fdManagerTest + # Perl module tests: TESTS += macLib diff --git a/modules/libcom/test/epicsRunLibComTests.c b/modules/libcom/test/epicsRunLibComTests.c index 54c46a68e..d560fd46a 100644 --- a/modules/libcom/test/epicsRunLibComTests.c +++ b/modules/libcom/test/epicsRunLibComTests.c @@ -54,6 +54,7 @@ int initHookTest(void); int ipAddrToAsciiTest(void); int macDefExpandTest(void); int macLibTest(void); +int fdManagerTest(void); int osiSockTest(void); int ringBytesTest(void); int ringPointerTest(void); @@ -110,6 +111,7 @@ void epicsRunLibComTests(void) runTest(ipAddrToAsciiTest); runTest(macDefExpandTest); runTest(macLibTest); + runTest(fdManagerTest); runTest(osiSockTest); runTest(ringBytesTest); runTest(ringPointerTest); diff --git a/modules/libcom/test/fdManagerTest.cpp b/modules/libcom/test/fdManagerTest.cpp new file mode 100644 index 000000000..49c1e96ce --- /dev/null +++ b/modules/libcom/test/fdManagerTest.cpp @@ -0,0 +1,312 @@ +/*************************************************************************\ +* Copyright (c) 2025 Michael Davidsaver +* SPDX-License-Identifier: EPICS +* EPICS Base is distributed subject to a Software License Agreement found +* in file LICENSE that is included with this distribution. +\*************************************************************************/ + +#include + +#include +#include +#include +#include +#include + +#include +#include + +#if __cplusplus<201103L +# define final +# define override +#endif + +namespace { + +void set_non_blocking(SOCKET sd) +{ + osiSockIoctl_t yes = true; + int status = socket_ioctl ( sd, + FIONBIO, & yes); + if(status) + testFail("set_non_blocking fails : %d", SOCKERRNO); +} + +// RAII for epicsTimer +struct ScopedTimer { + epicsTimer& timer; + ScopedTimer(epicsTimer& t) :timer(t) {} + ~ScopedTimer() { timer.destroy(); } +}; +struct ScopedFDReg { + fdReg * const reg; + ScopedFDReg(fdReg* reg) :reg(reg) {} + ~ScopedFDReg() { reg->destroy(); } +}; + +// RAII for socket +struct Socket { + SOCKET sd; + Socket() :sd(INVALID_SOCKET) {} + explicit + Socket(SOCKET sd) :sd(sd) {} + Socket(int af, int type) + :sd(epicsSocketCreate(af, type, 0)) + { + if(sd==INVALID_SOCKET) + testAbort("failed to allocate socket %d %d", af, type); + } +private: + Socket(const Socket&); + Socket& operator=(const Socket&); +public: + ~Socket() { + if(sd!=INVALID_SOCKET) + epicsSocketDestroy(sd); + } + void swap(Socket& o) { + std::swap(sd, o.sd); + } + sockaddr_in bind() + { + sockaddr_in addr; + memset(&addr, 0, sizeof(addr)); + addr.sin_family = AF_INET; + addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + + if(::bind(sd, (sockaddr*)&addr, sizeof(addr))) + testAbort("Unable to bind lo : %d", SOCKERRNO); + + sockaddr_in ret; + osiSocklen_t addrlen = sizeof(ret); + if(getsockname(sd, (sockaddr*)&ret, &addrlen)) + testAbort("Unable to getsockname : %d", SOCKERRNO); + (void)addrlen; + + if(listen(sd, 1)) + testAbort("Unable to listen : %d", SOCKERRNO); + + return ret; + } +}; + +struct DoConnect final : public epicsThreadRunable { + const SOCKET sd; + sockaddr_in to; + DoConnect(SOCKET sd, const sockaddr_in& to) + :sd(sd) + ,to(to) + {} + + void run() override final { + int err = connect(sd, (sockaddr*)&to, sizeof(to)); + testOk(err==0, "connect() %d %d", err, SOCKERRNO); + } +}; + +struct DoAccept final : public epicsThreadRunable { + const SOCKET sd; + Socket peer; + sockaddr_in peer_addr; + DoAccept(SOCKET sd) :sd(sd) {} + void run() override final { + osiSocklen_t len(sizeof(peer_addr)); + Socket temp(accept(sd, (sockaddr*)&peer_addr, &len)); + if(temp.sd==INVALID_SOCKET) + testFail("accept() -> %d", SOCKERRNO); + temp.swap(peer); + } +}; + +struct DoRead final : public epicsThreadRunable { + const SOCKET sd; + void* buf; + unsigned buflen; + int n; + DoRead(SOCKET sd, void* buf, unsigned buflen): sd(sd), buf(buf), buflen(buflen), n(0) {} + void run() override final { + n = recv(sd, (char*)buf, buflen, 0); + if(n<0) + testFail("read() -> %d, %d", n, SOCKERRNO); + } +}; + +struct DoWriteAll final : public epicsThreadRunable { + const SOCKET sd; + const void* buf; + unsigned buflen; + DoWriteAll(SOCKET sd, const void* buf, unsigned buflen): sd(sd), buf(buf), buflen(buflen) {} + void run() override final { + unsigned nsent = 0; + while(nsent %d, %d", n, SOCKERRNO); + break; + } + nsent += n; + } + } +}; + +struct Expire final : public epicsTimerNotify { + bool expired; + + Expire() :expired(false) {} + virtual ~Expire() {} + virtual expireStatus expire(const epicsTime &) override final + { + if(!expired) { + expired = true; + testPass("expired"); + } else { + testFail("re-expired?"); + } + return noRestart; + } +}; + +struct Event final : public fdReg { + int* ready; + + Event(fdManager& mgr, fdRegType evt, SOCKET sd, int* ready) + :fdReg(sd, evt, false, mgr) + ,ready(ready) + {} + virtual ~Event() {} + + virtual void callBack() override final { + epics::atomic::set(*ready, 1); + } +}; + +struct OneShot final : public fdReg { + int *mask; + + OneShot(fdManager& mgr, fdRegType evt, SOCKET sd, int *mask) + :fdReg(sd, evt, true, mgr) + ,mask(mask) + {} + virtual ~OneShot() { + epics::atomic::add(*mask, 2); + } + + virtual void callBack() override final { + epics::atomic::add(*mask, 1); + } +}; + +void testEmpty() +{ + fdManager empty; + empty.process(0.1); // ca-gateway always passes 0.01 + testPass("Did nothing"); +} + +void testOnlyTimer() +{ + fdManager mgr; + Expire trig, never; + ScopedTimer trig_timer(mgr.createTimer()), + never_timer(mgr.createTimer()); + epicsTime now(epicsTime::getCurrent()); + trig_timer.timer.start(trig, now+0.1); + never_timer.timer.start(never, now+9999999.0); + mgr.process(0.2); + testOk1(trig.expired); + testOk1(!never.expired); +} + +void testSockIO() +{ + fdManager mgr; + Socket listener(AF_INET, SOCK_STREAM); + set_non_blocking(listener.sd); + sockaddr_in servAddr(listener.bind()); + + Socket client(AF_INET, SOCK_STREAM); + Socket server; + // listen() / connect() + { + int readable = 0; + Event evt(mgr, fdrRead, listener.sd, &readable); + DoConnect conn(client.sd, servAddr); + epicsThread connector(conn, "connect", 0); + connector.start(); + + mgr.process(5.0); + + testOk1(readable); + + DoAccept acc(listener.sd); + acc.run(); + server.swap(acc.peer); + } + set_non_blocking(server.sd); + // writeable + { + int mask = 0; + new OneShot(mgr, fdrWrite, server.sd, &mask); + + mgr.process(5.0); + + testOk(mask==3, "OneShot event mask %x", mask); + } + // read + { + const char msg[] = "testing"; + int readable = 0; + Event evt(mgr, fdrRead, server.sd, &readable); + DoWriteAll op(client.sd, msg, sizeof(msg)-1); + epicsThread writer(op, "writer", 0); + writer.start(); + + mgr.process(5.0); + + testOk1(readable); + + char buf[sizeof(msg)] = ""; + DoRead(server.sd, buf, sizeof(buf)-1).run(); + buf[sizeof(buf)-1] = '\0'; + testOk(strcmp(msg, buf)==0, "%s == %s", msg, buf); + } + // timer while unreadable + { + + int readable = 0; + Event evt(mgr, fdrRead, server.sd, &readable); + Expire tmo; + ScopedTimer timer(mgr.createTimer()); + timer.timer.start(tmo, epicsTime::getCurrent()); // immediate + + mgr.process(1.0); + + testOk1(!readable); + testOk1(tmo.expired); + } + // notification on close() + { + int readable = 0; + Event evt(mgr, fdrRead, server.sd, &readable); + + shutdown(client.sd, SHUT_RDWR); + //Socket().swap(client); + + mgr.process(1.0); + + testOk1(readable); + } +} + +} // namespace + +MAIN(fdManagerTest) +{ + testPlan(13); + osiSockAttach(); + testEmpty(); + testOnlyTimer(); + testSockIO(); + osiSockRelease(); + return testDone(); +} From 57c02950243bca673d98e591fdd3e6d37cb44c03 Mon Sep 17 00:00:00 2001 From: Dirk Zimoch Date: Mon, 20 Jan 2025 09:30:27 +0100 Subject: [PATCH 03/15] fdManager changed to use poll() The implementation using select() limits file desciptors to FD_SETSIZE, typically 1024 on Linux. This number is too low for some applications, for example for the CA gateway. Therefore, Linux builds use poll() instead. --- modules/libcom/src/fdmgr/fdManager.cpp | 122 +++++++++++++++++++++---- modules/libcom/src/fdmgr/fdManager.h | 26 +++++- 2 files changed, 128 insertions(+), 20 deletions(-) diff --git a/modules/libcom/src/fdmgr/fdManager.cpp b/modules/libcom/src/fdmgr/fdManager.cpp index 8a9ed788f..94c5f7b14 100644 --- a/modules/libcom/src/fdmgr/fdManager.cpp +++ b/modules/libcom/src/fdmgr/fdManager.cpp @@ -31,27 +31,41 @@ using std :: max; fdManager fileDescriptorManager; -const unsigned mSecPerSec = 1000u; -const unsigned uSecPerSec = 1000u * mSecPerSec; +static const unsigned mSecPerSec = 1000u; +static const unsigned uSecPerSec = 1000u * mSecPerSec; + +#ifdef FDMGR_USE_POLL +static const int PollEvents[] = { // must match fdRegType + POLLRDNORM | POLLRDBAND | POLLIN | POLLHUP | POLLERR, + POLLWRBAND | POLLWRNORM | POLLOUT | POLLERR, + POLLPRI}; +#endif // // fdManager::fdManager() // -// hopefully its a reasonable guess that select() and epicsThreadSleep() +// hopefully its a reasonable guess that poll()/select() and epicsThreadSleep() // will have the same sleep quantum // -LIBCOM_API fdManager::fdManager () : +LIBCOM_API fdManager::fdManager () : sleepQuantum ( epicsThreadSleepQuantum () ), - fdSetsPtr ( new fd_set [fdrNEnums] ), - pTimerQueue ( 0 ), maxFD ( 0 ), processInProg ( false ), - pCBReg ( 0 ) + pTimerQueue ( 0 ), processInProg ( false ), +#ifdef FDMGR_USE_POLL + nfds ( 0 ), pollfdsCap ( 0 ), pollfds ( 0 ), +#endif +#ifdef FDMGR_USE_SELECT + fdSetsPtr ( new fd_set [fdrNEnums] ), maxFD ( 0 ), +#endif + pCBReg ( 0 ) { int status = osiSockAttach (); assert (status); +#ifdef FDMGR_USE_SELECT for ( size_t i = 0u; i < fdrNEnums; i++ ) { FD_ZERO ( &fdSetsPtr[i] ); } +#endif } // @@ -70,7 +84,12 @@ LIBCOM_API fdManager::~fdManager() pReg->destroy(); } delete this->pTimerQueue; +#ifdef FDMGR_USE_POLL + delete [] this->pollfds; +#endif +#ifdef FDMGR_USE_SELECT delete [] this->fdSetsPtr; +#endif osiSockRelease(); } @@ -91,7 +110,7 @@ LIBCOM_API void fdManager::process (double delay) // // One shot at expired timers prior to going into - // select. This allows zero delay timers to arm + // poll/select. This allows zero delay timers to arm // fd writes. We will never process the timer queue // more than once here so that fd activity get serviced // in a reasonable length of time. @@ -102,15 +121,30 @@ LIBCOM_API void fdManager::process (double delay) minDelay = delay; } - bool ioPending = false; + int ioPending = 0; tsDLIter < fdReg > iter = this->regList.firstIter (); while ( iter.valid () ) { +#ifdef FDMGR_USE_POLL + pollfds[ioPending].fd = iter->getFD(); + pollfds[ioPending].events = PollEvents[iter->getType()]; +#endif +#ifdef FDMGR_USE_SELECT FD_SET(iter->getFD(), &this->fdSetsPtr[iter->getType()]); - ioPending = true; +#endif + ++ioPending; ++iter; } if ( ioPending ) { +#ifdef FDMGR_USE_POLL + if (minDelay * mSecPerSec > INT_MAX) + minDelay = INT_MAX / mSecPerSec; + + int status = poll(pollfds, ioPending, static_cast(minDelay * mSecPerSec)); + int i = 0; +#endif + +#ifdef FDMGR_USE_SELECT struct timeval tv; tv.tv_sec = static_cast ( minDelay ); tv.tv_usec = static_cast ( (minDelay-tv.tv_sec) * uSecPerSec ); @@ -119,6 +153,7 @@ LIBCOM_API void fdManager::process (double delay) fd_set * pWriteSet = & this->fdSetsPtr[fdrWrite]; fd_set * pExceptSet = & this->fdSetsPtr[fdrException]; int status = select (this->maxFD, pReadSet, pWriteSet, pExceptSet, &tv); +#endif this->pTimerQueue->process(epicsTime::getCurrent()); @@ -131,8 +166,31 @@ LIBCOM_API void fdManager::process (double delay) while ( iter.valid () && status > 0 ) { tsDLIter < fdReg > tmp = iter; tmp++; + +#ifdef FDMGR_USE_POLL + // In a single threaded application, nothing should have + // changed the order of regList and pollfds by now. + // But just in case... + int isave = i; + while (pollfds[i].fd != iter->getFD() || pollfds[i].events != PollEvents[iter->getType()]) + { + i++; // skip pollfd of removed items + if (i >= ioPending) { // skip unknown (inserted?) items + iter = tmp; + tmp++; + if (!iter.valid()) break; + i = isave; + } + } + if (i >= ioPending) break; // any unhandled item stays in regList for next time + + if (pollfds[i++].revents & PollEvents[iter->getType()]) { +#endif + +#ifdef FDMGR_USE_SELECT if (FD_ISSET(iter->getFD(), &this->fdSetsPtr[iter->getType()])) { FD_CLR(iter->getFD(), &this->fdSetsPtr[iter->getType()]); +#endif this->regList.remove(*iter); this->activeList.add(*iter); iter->state = fdReg::active; @@ -177,22 +235,30 @@ LIBCOM_API void fdManager::process (double delay) else if ( status < 0 ) { int errnoCpy = SOCKERRNO; +#ifdef FDMGR_USE_SELECT // don't depend on flags being properly set if // an error is returned from select for ( size_t i = 0u; i < fdrNEnums; i++ ) { FD_ZERO ( &fdSetsPtr[i] ); } +#endif // - // print a message if its an unexpected error + // print a message if it's an unexpected error // if ( errnoCpy != SOCK_EINTR ) { char sockErrBuf[64]; epicsSocketConvertErrnoToString ( sockErrBuf, sizeof ( sockErrBuf ) ); - fprintf ( stderr, - "fdManager: select failed because \"%s\"\n", - sockErrBuf ); + errlogPrintf("fdManager: " +#ifdef FDMGR_USE_POLL + "poll()" +#endif +#ifdef FDMGR_USE_SELECT + "select()" +#endif + " failed because \"%s\"\n", + sockErrBuf); } } } @@ -256,8 +322,10 @@ void fdRegId::show ( unsigned level ) const // void fdManager::installReg (fdReg ®) { +#ifdef FDMGR_USE_SELECT this->maxFD = max ( this->maxFD, reg.getFD()+1 ); - // Most applications will find that its important to push here to +#endif + // Most applications will find that it's important to push here to // the front of the list so that transient writes get executed // first allowing incoming read protocol to find that outgoing // buffer space is newly available. @@ -268,6 +336,17 @@ void fdManager::installReg (fdReg ®) if ( status != 0 ) { throwWithLocation ( fdInterestSubscriptionAlreadyExits () ); } +#ifdef FDMGR_USE_POLL + // keep enough event slots for all fds to become available at once + if (++nfds > pollfdsCap) { + if (pollfdsCap == 0) + pollfdsCap = 16; + else + pollfdsCap *= 2; + delete[] pollfds; + pollfds = new struct pollfd[pollfdsCap]; + } +#endif } // @@ -279,8 +358,7 @@ void fdManager::removeReg (fdReg ®In) pItemFound = this->fdTbl.remove (regIn); if (pItemFound!=®In) { - fprintf(stderr, - "fdManager::removeReg() bad fd registration object\n"); + errlogPrintf("fdManager::removeReg() bad fd registration object\n"); return; } @@ -309,7 +387,13 @@ void fdManager::removeReg (fdReg ®In) } regIn.state = fdReg::limbo; +#ifdef FDMGR_USE_SELECT FD_CLR(regIn.getFD(), &this->fdSetsPtr[regIn.getType()]); +#endif + +#ifdef FDMGR_USE_POLL + nfds--; +#endif } // @@ -346,11 +430,13 @@ fdReg::fdReg (const SOCKET fdIn, const fdRegType typIn, fdRegId (fdIn,typIn), state (limbo), onceOnly (onceOnlyIn), manager (managerIn) { +#ifdef FDMGR_USE_SELECT if (!FD_IN_FDSET(fdIn)) { - fprintf (stderr, "%s: fd > FD_SETSIZE ignored\n", + errlogPrintf("%s: fd > FD_SETSIZE ignored\n", __FILE__); return; } +#endif this->manager.installReg (*this); } diff --git a/modules/libcom/src/fdmgr/fdManager.h b/modules/libcom/src/fdmgr/fdManager.h index 3112b26f4..22fce9762 100644 --- a/modules/libcom/src/fdmgr/fdManager.h +++ b/modules/libcom/src/fdmgr/fdManager.h @@ -19,6 +19,18 @@ #ifndef fdManagerH_included #define fdManagerH_included +#if !defined(FDMGR_USE_POLL) && !defined(FDMGR_USE_SELECT) +#if defined(__linux__) +#define FDMGR_USE_POLL +#else +#define FDMGR_USE_SELECT +#endif +#endif + +#ifdef FDMGR_USE_POLL +#include +#endif + #include "libComAPI.h" // reset share lib defines #include "tsDLList.h" #include "resourceLib.h" @@ -91,10 +103,20 @@ private: tsDLList < fdReg > activeList; resTable < fdReg, fdRegId > fdTbl; const double sleepQuantum; - fd_set * fdSetsPtr; epicsTimerQueuePassive * pTimerQueue; - SOCKET maxFD; bool processInProg; + +#ifdef FDMGR_USE_POLL + int nfds; + int pollfdsCap; + struct pollfd *pollfds; +#endif + +#ifdef FDMGR_USE_SELECT + fd_set * fdSetsPtr; + SOCKET maxFD; +#endif + // // Set to fdreg when in call back // and nill otherwise From cbbbd6784301fefec1894b222e90bb4a1a5ea67d Mon Sep 17 00:00:00 2001 From: Dirk Zimoch Date: Thu, 23 Jan 2025 10:30:38 +0100 Subject: [PATCH 04/15] fdManager uses poll() on Windows and RTEMS too RTEMS needs to use the "new" network stack Windows has poll since Vista Don't use poll on cygwin: it emulates poll() using select(). --- modules/libcom/src/fdmgr/fdManager.cpp | 23 ++++++++++++++--------- modules/libcom/src/fdmgr/fdManager.h | 24 ++++++++++++------------ 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/modules/libcom/src/fdmgr/fdManager.cpp b/modules/libcom/src/fdmgr/fdManager.cpp index 94c5f7b14..d0f416422 100644 --- a/modules/libcom/src/fdmgr/fdManager.cpp +++ b/modules/libcom/src/fdmgr/fdManager.cpp @@ -19,28 +19,33 @@ // 1) This library is not thread safe // -#include - #define instantiateRecourceLib #include "epicsAssert.h" #include "epicsThread.h" #include "fdManager.h" #include "locationException.h" -using std :: max; - -fdManager fileDescriptorManager; - -static const unsigned mSecPerSec = 1000u; -static const unsigned uSecPerSec = 1000u * mSecPerSec; - #ifdef FDMGR_USE_POLL +#ifdef _WIN32 +#define poll WSAPoll +#endif + static const int PollEvents[] = { // must match fdRegType POLLRDNORM | POLLRDBAND | POLLIN | POLLHUP | POLLERR, POLLWRBAND | POLLWRNORM | POLLOUT | POLLERR, POLLPRI}; #endif +#ifdef FDMGR_USE_SELECT +#include +using std :: max; +#endif + +fdManager fileDescriptorManager; + +static const unsigned mSecPerSec = 1000u; +static const unsigned uSecPerSec = 1000u * mSecPerSec; + // // fdManager::fdManager() // diff --git a/modules/libcom/src/fdmgr/fdManager.h b/modules/libcom/src/fdmgr/fdManager.h index 22fce9762..7665b6c3e 100644 --- a/modules/libcom/src/fdmgr/fdManager.h +++ b/modules/libcom/src/fdmgr/fdManager.h @@ -19,18 +19,6 @@ #ifndef fdManagerH_included #define fdManagerH_included -#if !defined(FDMGR_USE_POLL) && !defined(FDMGR_USE_SELECT) -#if defined(__linux__) -#define FDMGR_USE_POLL -#else -#define FDMGR_USE_SELECT -#endif -#endif - -#ifdef FDMGR_USE_POLL -#include -#endif - #include "libComAPI.h" // reset share lib defines #include "tsDLList.h" #include "resourceLib.h" @@ -38,6 +26,18 @@ #include "osiSock.h" #include "epicsTimer.h" +#if !defined(FDMGR_USE_POLL) && !defined(FDMGR_USE_SELECT) +#if defined(__linux__) || _WIN32_WINNT >= 0x600 || (defined(__rtems__) && !defined(RTEMS_LEGACY_STACK)) +#define FDMGR_USE_POLL +#else +#define FDMGR_USE_SELECT +#endif +#endif + +#if defined(FDMGR_USE_POLL) && !defined(_WIN32) +#include +#endif + enum fdRegType {fdrRead, fdrWrite, fdrException, fdrNEnums}; // From ece031c88bfc6bca90701f1bfd17280c7eebf501 Mon Sep 17 00:00:00 2001 From: Dirk Zimoch Date: Fri, 24 Jan 2025 11:59:13 +0100 Subject: [PATCH 05/15] fdManager use std::vector --- modules/libcom/src/fdmgr/fdManager.cpp | 44 +++++++++++--------------- modules/libcom/src/fdmgr/fdManager.h | 9 +++--- 2 files changed, 24 insertions(+), 29 deletions(-) diff --git a/modules/libcom/src/fdmgr/fdManager.cpp b/modules/libcom/src/fdmgr/fdManager.cpp index d0f416422..7b1b128a2 100644 --- a/modules/libcom/src/fdmgr/fdManager.cpp +++ b/modules/libcom/src/fdmgr/fdManager.cpp @@ -30,7 +30,7 @@ #define poll WSAPoll #endif -static const int PollEvents[] = { // must match fdRegType +static const short PollEvents[] = { // must match fdRegType POLLRDNORM | POLLRDBAND | POLLIN | POLLHUP | POLLERR, POLLWRBAND | POLLWRNORM | POLLOUT | POLLERR, POLLPRI}; @@ -55,9 +55,6 @@ static const unsigned uSecPerSec = 1000u * mSecPerSec; LIBCOM_API fdManager::fdManager () : sleepQuantum ( epicsThreadSleepQuantum () ), pTimerQueue ( 0 ), processInProg ( false ), -#ifdef FDMGR_USE_POLL - nfds ( 0 ), pollfdsCap ( 0 ), pollfds ( 0 ), -#endif #ifdef FDMGR_USE_SELECT fdSetsPtr ( new fd_set [fdrNEnums] ), maxFD ( 0 ), #endif @@ -89,9 +86,6 @@ LIBCOM_API fdManager::~fdManager() pReg->destroy(); } delete this->pTimerQueue; -#ifdef FDMGR_USE_POLL - delete [] this->pollfds; -#endif #ifdef FDMGR_USE_SELECT delete [] this->fdSetsPtr; #endif @@ -126,17 +120,30 @@ LIBCOM_API void fdManager::process (double delay) minDelay = delay; } +#ifdef FDMGR_USE_POLL + pollfds.clear(); +#endif + int ioPending = 0; tsDLIter < fdReg > iter = this->regList.firstIter (); while ( iter.valid () ) { + ++ioPending; + #ifdef FDMGR_USE_POLL - pollfds[ioPending].fd = iter->getFD(); - pollfds[ioPending].events = PollEvents[iter->getType()]; +#if __cplusplus >= 201100L + pollfds.emplace_back(pollfd{.fd = iter->getFD(), .events = PollEvents[iter->getType()]}); +#else + struct pollfd pollfd; + pollfd.fd = iter->getFD(); + pollfd.events = PollEvents[iter->getType()]; + pollfd.revents = 0; + pollfds.push_back(pollfd); #endif +#endif + #ifdef FDMGR_USE_SELECT FD_SET(iter->getFD(), &this->fdSetsPtr[iter->getType()]); #endif - ++ioPending; ++iter; } @@ -145,7 +152,8 @@ LIBCOM_API void fdManager::process (double delay) if (minDelay * mSecPerSec > INT_MAX) minDelay = INT_MAX / mSecPerSec; - int status = poll(pollfds, ioPending, static_cast(minDelay * mSecPerSec)); + int status = poll(&pollfds.front(), // ancient C++ has no vector.data() + ioPending, static_cast(minDelay * mSecPerSec)); int i = 0; #endif @@ -341,17 +349,6 @@ void fdManager::installReg (fdReg ®) if ( status != 0 ) { throwWithLocation ( fdInterestSubscriptionAlreadyExits () ); } -#ifdef FDMGR_USE_POLL - // keep enough event slots for all fds to become available at once - if (++nfds > pollfdsCap) { - if (pollfdsCap == 0) - pollfdsCap = 16; - else - pollfdsCap *= 2; - delete[] pollfds; - pollfds = new struct pollfd[pollfdsCap]; - } -#endif } // @@ -396,9 +393,6 @@ void fdManager::removeReg (fdReg ®In) FD_CLR(regIn.getFD(), &this->fdSetsPtr[regIn.getType()]); #endif -#ifdef FDMGR_USE_POLL - nfds--; -#endif } // diff --git a/modules/libcom/src/fdmgr/fdManager.h b/modules/libcom/src/fdmgr/fdManager.h index 7665b6c3e..a323ee1bb 100644 --- a/modules/libcom/src/fdmgr/fdManager.h +++ b/modules/libcom/src/fdmgr/fdManager.h @@ -34,9 +34,12 @@ #endif #endif -#if defined(FDMGR_USE_POLL) && !defined(_WIN32) +#if defined(FDMGR_USE_POLL) +#include +#if !defined(_WIN32) #include #endif +#endif enum fdRegType {fdrRead, fdrWrite, fdrException, fdrNEnums}; @@ -107,9 +110,7 @@ private: bool processInProg; #ifdef FDMGR_USE_POLL - int nfds; - int pollfdsCap; - struct pollfd *pollfds; + std::vector pollfds; #endif #ifdef FDMGR_USE_SELECT From f09b235fced37bbf4e25ecdd7463c78c29d0d3ef Mon Sep 17 00:00:00 2001 From: Dirk Zimoch Date: Fri, 24 Jan 2025 17:45:46 +0100 Subject: [PATCH 06/15] Keep implementation details of fdManager out of header file --- modules/libcom/src/fdmgr/fdManager.cpp | 166 ++++++++++++++++--------- modules/libcom/src/fdmgr/fdManager.h | 60 +-------- 2 files changed, 114 insertions(+), 112 deletions(-) diff --git a/modules/libcom/src/fdmgr/fdManager.cpp b/modules/libcom/src/fdmgr/fdManager.cpp index 7b1b128a2..d71160e1f 100644 --- a/modules/libcom/src/fdmgr/fdManager.cpp +++ b/modules/libcom/src/fdmgr/fdManager.cpp @@ -25,9 +25,20 @@ #include "fdManager.h" #include "locationException.h" +#if !defined(FDMGR_USE_POLL) && !defined(FDMGR_USE_SELECT) +#if defined(__linux__) || _WIN32_WINNT >= 0x600 || (defined(__rtems__) && !defined(RTEMS_LEGACY_STACK)) +#define FDMGR_USE_POLL +#else +#define FDMGR_USE_SELECT +#endif +#endif + #ifdef FDMGR_USE_POLL -#ifdef _WIN32 +#include +#if defined(_WIN32) #define poll WSAPoll +#else +#include #endif static const short PollEvents[] = { // must match fdRegType @@ -38,9 +49,55 @@ static const short PollEvents[] = { // must match fdRegType #ifdef FDMGR_USE_SELECT #include -using std :: max; #endif +struct fdManagerPrivate { + tsDLList < fdReg > regList; + tsDLList < fdReg > activeList; + resTable < fdReg, fdRegId > fdTbl; + const double sleepQuantum; + epicsTimerQueuePassive * pTimerQueue; + bool processInProg; + +#ifdef FDMGR_USE_POLL + std::vector pollfds; +#endif + +#ifdef FDMGR_USE_SELECT + fd_set fdSets[fdrNEnums]; + SOCKET maxFD; +#endif + + // + // Set to fdreg when in call back + // and nill otherwise + // + fdReg * pCBReg; + fdManager & owner; + + fdManagerPrivate(fdManager & owner); + void lazyInitTimerQueue(); +}; + +fdManagerPrivate::fdManagerPrivate(fdManager & owner) : + sleepQuantum(epicsThreadSleepQuantum()), + pTimerQueue(0), processInProg(false), + pCBReg(0), owner(owner) +{} + +inline void fdManagerPrivate::lazyInitTimerQueue () +{ + if (!pTimerQueue) { + pTimerQueue = & epicsTimerQueuePassive::create(owner); + } +} + +epicsTimer & fdManager::createTimer() +{ + priv->lazyInitTimerQueue(); + return priv->pTimerQueue->createTimer(); +} + fdManager fileDescriptorManager; static const unsigned mSecPerSec = 1000u; @@ -53,19 +110,15 @@ static const unsigned uSecPerSec = 1000u * mSecPerSec; // will have the same sleep quantum // LIBCOM_API fdManager::fdManager () : - sleepQuantum ( epicsThreadSleepQuantum () ), - pTimerQueue ( 0 ), processInProg ( false ), -#ifdef FDMGR_USE_SELECT - fdSetsPtr ( new fd_set [fdrNEnums] ), maxFD ( 0 ), -#endif - pCBReg ( 0 ) + priv(new fdManagerPrivate(*this)) { int status = osiSockAttach (); assert (status); #ifdef FDMGR_USE_SELECT + priv->maxFD = 0; for ( size_t i = 0u; i < fdrNEnums; i++ ) { - FD_ZERO ( &fdSetsPtr[i] ); + FD_ZERO(&priv->fdSets[i]); } #endif } @@ -77,18 +130,15 @@ LIBCOM_API fdManager::~fdManager() { fdReg *pReg; - while ( (pReg = this->regList.get()) ) { + while ((pReg = priv->regList.get())) { pReg->state = fdReg::limbo; pReg->destroy(); } - while ( (pReg = this->activeList.get()) ) { + while ((pReg = priv->activeList.get())) { pReg->state = fdReg::limbo; pReg->destroy(); } - delete this->pTimerQueue; -#ifdef FDMGR_USE_SELECT - delete [] this->fdSetsPtr; -#endif + delete priv->pTimerQueue; osiSockRelease(); } @@ -97,15 +147,14 @@ LIBCOM_API fdManager::~fdManager() // LIBCOM_API void fdManager::process (double delay) { - this->lazyInitTimerQueue (); + priv->lazyInitTimerQueue (); // // no recursion // - if (this->processInProg) { + if (priv->processInProg) return; - } - this->processInProg = true; + priv->processInProg = true; // // One shot at expired timers prior to going into @@ -114,35 +163,35 @@ LIBCOM_API void fdManager::process (double delay) // more than once here so that fd activity get serviced // in a reasonable length of time. // - double minDelay = this->pTimerQueue->process(epicsTime::getCurrent()); + double minDelay = priv->pTimerQueue->process(epicsTime::getCurrent()); if ( minDelay >= delay ) { minDelay = delay; } #ifdef FDMGR_USE_POLL - pollfds.clear(); + priv->pollfds.clear(); #endif int ioPending = 0; - tsDLIter < fdReg > iter = this->regList.firstIter (); + tsDLIter iter = priv->regList.firstIter(); while ( iter.valid () ) { ++ioPending; #ifdef FDMGR_USE_POLL #if __cplusplus >= 201100L - pollfds.emplace_back(pollfd{.fd = iter->getFD(), .events = PollEvents[iter->getType()]}); + priv->pollfds.emplace_back(pollfd{.fd = iter->getFD(), .events = PollEvents[iter->getType()]}); #else struct pollfd pollfd; pollfd.fd = iter->getFD(); pollfd.events = PollEvents[iter->getType()]; pollfd.revents = 0; - pollfds.push_back(pollfd); + priv->pollfds.push_back(pollfd); #endif #endif #ifdef FDMGR_USE_SELECT - FD_SET(iter->getFD(), &this->fdSetsPtr[iter->getType()]); + FD_SET(iter->getFD(), &priv->fdSets[iter->getType()]); #endif ++iter; } @@ -152,7 +201,7 @@ LIBCOM_API void fdManager::process (double delay) if (minDelay * mSecPerSec > INT_MAX) minDelay = INT_MAX / mSecPerSec; - int status = poll(&pollfds.front(), // ancient C++ has no vector.data() + int status = poll(&priv->pollfds.front(), // ancient C++ has no vector.data() ioPending, static_cast(minDelay * mSecPerSec)); int i = 0; #endif @@ -162,20 +211,20 @@ LIBCOM_API void fdManager::process (double delay) tv.tv_sec = static_cast ( minDelay ); tv.tv_usec = static_cast ( (minDelay-tv.tv_sec) * uSecPerSec ); - fd_set * pReadSet = & this->fdSetsPtr[fdrRead]; - fd_set * pWriteSet = & this->fdSetsPtr[fdrWrite]; - fd_set * pExceptSet = & this->fdSetsPtr[fdrException]; - int status = select (this->maxFD, pReadSet, pWriteSet, pExceptSet, &tv); + int status = select(priv->maxFD, + &priv->fdSets[fdrRead], + &priv->fdSets[fdrWrite], + &priv->fdSets[fdrException], &tv); #endif - this->pTimerQueue->process(epicsTime::getCurrent()); + priv->pTimerQueue->process(epicsTime::getCurrent()); if ( status > 0 ) { // // Look for activity // - iter=this->regList.firstIter (); + iter = priv->regList.firstIter (); while ( iter.valid () && status > 0 ) { tsDLIter < fdReg > tmp = iter; tmp++; @@ -185,7 +234,8 @@ LIBCOM_API void fdManager::process (double delay) // changed the order of regList and pollfds by now. // But just in case... int isave = i; - while (pollfds[i].fd != iter->getFD() || pollfds[i].events != PollEvents[iter->getType()]) + while (priv->pollfds[i].fd != iter->getFD() || + priv->pollfds[i].events != PollEvents[iter->getType()]) { i++; // skip pollfd of removed items if (i >= ioPending) { // skip unknown (inserted?) items @@ -197,15 +247,15 @@ LIBCOM_API void fdManager::process (double delay) } if (i >= ioPending) break; // any unhandled item stays in regList for next time - if (pollfds[i++].revents & PollEvents[iter->getType()]) { + if (priv->pollfds[i++].revents & PollEvents[iter->getType()]) { #endif #ifdef FDMGR_USE_SELECT - if (FD_ISSET(iter->getFD(), &this->fdSetsPtr[iter->getType()])) { - FD_CLR(iter->getFD(), &this->fdSetsPtr[iter->getType()]); + if (FD_ISSET(iter->getFD(), &priv->fdSets[iter->getType()])) { + FD_CLR(iter->getFD(), &priv->fdSets[iter->getType()]); #endif - this->regList.remove(*iter); - this->activeList.add(*iter); + priv->regList.remove(*iter); + priv->activeList.add(*iter); iter->state = fdReg::active; status--; } @@ -217,7 +267,7 @@ LIBCOM_API void fdManager::process (double delay) // above list while in a "callBack()" routine // fdReg * pReg; - while ( (pReg = this->activeList.get()) ) { + while ((pReg = priv->activeList.get())) { pReg->state = fdReg::limbo; // @@ -225,21 +275,21 @@ LIBCOM_API void fdManager::process (double delay) // can detect if it was deleted // during the call back // - this->pCBReg = pReg; + priv->pCBReg = pReg; pReg->callBack(); - if (this->pCBReg != NULL) { + if (priv->pCBReg != NULL) { // // check only after we see that it is non-null so // that we don't trigger bounds-checker dangling pointer // error // - assert (this->pCBReg==pReg); - this->pCBReg = 0; + assert(priv->pCBReg == pReg); + priv->pCBReg = NULL; if (pReg->onceOnly) { pReg->destroy(); } else { - this->regList.add(*pReg); + priv->regList.add(*pReg); pReg->state = fdReg::pending; } } @@ -252,7 +302,7 @@ LIBCOM_API void fdManager::process (double delay) // don't depend on flags being properly set if // an error is returned from select for ( size_t i = 0u; i < fdrNEnums; i++ ) { - FD_ZERO ( &fdSetsPtr[i] ); + FD_ZERO(&priv->fdSets[i]); } #endif @@ -282,9 +332,9 @@ LIBCOM_API void fdManager::process (double delay) * of select() */ epicsThreadSleep(minDelay); - this->pTimerQueue->process(epicsTime::getCurrent()); + priv->pTimerQueue->process(epicsTime::getCurrent()); } - this->processInProg = false; + priv->processInProg = false; } // @@ -336,16 +386,16 @@ void fdRegId::show ( unsigned level ) const void fdManager::installReg (fdReg ®) { #ifdef FDMGR_USE_SELECT - this->maxFD = max ( this->maxFD, reg.getFD()+1 ); + priv->maxFD = std::max(priv->maxFD, reg.getFD()+1); #endif // Most applications will find that it's important to push here to // the front of the list so that transient writes get executed // first allowing incoming read protocol to find that outgoing // buffer space is newly available. - this->regList.push ( reg ); + priv->regList.push(reg); reg.state = fdReg::pending; - int status = this->fdTbl.add ( reg ); + int status = priv->fdTbl.add(reg); if ( status != 0 ) { throwWithLocation ( fdInterestSubscriptionAlreadyExits () ); } @@ -358,7 +408,7 @@ void fdManager::removeReg (fdReg ®In) { fdReg *pItemFound; - pItemFound = this->fdTbl.remove (regIn); + pItemFound = priv->fdTbl.remove(regIn); if (pItemFound!=®In) { errlogPrintf("fdManager::removeReg() bad fd registration object\n"); return; @@ -368,16 +418,16 @@ void fdManager::removeReg (fdReg ®In) // signal fdManager that the fdReg was deleted // during the call back // - if (this->pCBReg == ®In) { - this->pCBReg = 0; + if (priv->pCBReg == ®In) { + priv->pCBReg = NULL; } switch (regIn.state) { case fdReg::active: - this->activeList.remove (regIn); + priv->activeList.remove(regIn); break; case fdReg::pending: - this->regList.remove (regIn); + priv->regList.remove(regIn); break; case fdReg::limbo: break; @@ -390,7 +440,7 @@ void fdManager::removeReg (fdReg ®In) regIn.state = fdReg::limbo; #ifdef FDMGR_USE_SELECT - FD_CLR(regIn.getFD(), &this->fdSetsPtr[regIn.getType()]); + FD_CLR(regIn.getFD(), &priv->fdSets[regIn.getType()]); #endif } @@ -406,7 +456,7 @@ void fdManager::reschedule () double fdManager::quantum () { - return this->sleepQuantum; + return priv->sleepQuantum; } // @@ -418,7 +468,7 @@ LIBCOM_API fdReg *fdManager::lookUpFD (const SOCKET fd, const fdRegType type) return NULL; } fdRegId id (fd,type); - return this->fdTbl.lookup(id); + return priv->fdTbl.lookup(id); } // diff --git a/modules/libcom/src/fdmgr/fdManager.h b/modules/libcom/src/fdmgr/fdManager.h index a323ee1bb..518539bc4 100644 --- a/modules/libcom/src/fdmgr/fdManager.h +++ b/modules/libcom/src/fdmgr/fdManager.h @@ -26,21 +26,6 @@ #include "osiSock.h" #include "epicsTimer.h" -#if !defined(FDMGR_USE_POLL) && !defined(FDMGR_USE_SELECT) -#if defined(__linux__) || _WIN32_WINNT >= 0x600 || (defined(__rtems__) && !defined(RTEMS_LEGACY_STACK)) -#define FDMGR_USE_POLL -#else -#define FDMGR_USE_SELECT -#endif -#endif - -#if defined(FDMGR_USE_POLL) -#include -#if !defined(_WIN32) -#include -#endif -#endif - enum fdRegType {fdrRead, fdrWrite, fdrException, fdrNEnums}; // @@ -85,49 +70,29 @@ private: // // file descriptor manager // -class fdManager : public epicsTimerQueueNotify { +class LIBCOM_API fdManager : public epicsTimerQueueNotify { public: // // exceptions // class fdInterestSubscriptionAlreadyExits {}; - LIBCOM_API fdManager (); - LIBCOM_API virtual ~fdManager (); - LIBCOM_API void process ( double delay ); // delay parameter is in seconds + fdManager (); + virtual ~fdManager (); + void process ( double delay ); // delay parameter is in seconds // returns NULL if the fd is unknown - LIBCOM_API class fdReg *lookUpFD (const SOCKET fd, const fdRegType type); + class fdReg *lookUpFD (const SOCKET fd, const fdRegType type); epicsTimer & createTimer (); private: - tsDLList < fdReg > regList; - tsDLList < fdReg > activeList; - resTable < fdReg, fdRegId > fdTbl; - const double sleepQuantum; - epicsTimerQueuePassive * pTimerQueue; - bool processInProg; + struct fdManagerPrivate* priv; -#ifdef FDMGR_USE_POLL - std::vector pollfds; -#endif - -#ifdef FDMGR_USE_SELECT - fd_set * fdSetsPtr; - SOCKET maxFD; -#endif - - // - // Set to fdreg when in call back - // and nill otherwise - // - fdReg * pCBReg; void reschedule (); double quantum (); void installReg (fdReg ®); void removeReg (fdReg ®); - void lazyInitTimerQueue (); fdManager ( const fdManager & ); fdManager & operator = ( const fdManager & ); friend class fdReg; @@ -210,18 +175,5 @@ inline resTableIndex fdRegId::hash () const return hashid; } -inline void fdManager::lazyInitTimerQueue () -{ - if ( ! this->pTimerQueue ) { - this->pTimerQueue = & epicsTimerQueuePassive::create ( *this ); - } -} - -inline epicsTimer & fdManager::createTimer () -{ - this->lazyInitTimerQueue (); - return this->pTimerQueue->createTimer (); -} - #endif // fdManagerH_included From bfc2f832ec1b184d0a591345eb915a43b3fe2c65 Mon Sep 17 00:00:00 2001 From: Dirk Zimoch Date: Wed, 29 Jan 2025 14:19:11 +0100 Subject: [PATCH 07/15] fdManager uses poll() on Darwin too --- modules/libcom/src/fdmgr/fdManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/libcom/src/fdmgr/fdManager.cpp b/modules/libcom/src/fdmgr/fdManager.cpp index d71160e1f..c6fa05fc5 100644 --- a/modules/libcom/src/fdmgr/fdManager.cpp +++ b/modules/libcom/src/fdmgr/fdManager.cpp @@ -26,7 +26,7 @@ #include "locationException.h" #if !defined(FDMGR_USE_POLL) && !defined(FDMGR_USE_SELECT) -#if defined(__linux__) || _WIN32_WINNT >= 0x600 || (defined(__rtems__) && !defined(RTEMS_LEGACY_STACK)) +#if defined(__linux__) || defined(darwin) || _WIN32_WINNT >= 0x600 || (defined(__rtems__) && !defined(RTEMS_LEGACY_STACK)) #define FDMGR_USE_POLL #else #define FDMGR_USE_SELECT From 5eb9997791d18cb66c8056250528db76b174d220 Mon Sep 17 00:00:00 2001 From: Dirk Zimoch Date: Thu, 30 Jan 2025 12:01:48 +0100 Subject: [PATCH 08/15] fix codacy warning: make fdManagerPrivate constructor explicit --- modules/libcom/src/fdmgr/fdManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/libcom/src/fdmgr/fdManager.cpp b/modules/libcom/src/fdmgr/fdManager.cpp index c6fa05fc5..58917b4c7 100644 --- a/modules/libcom/src/fdmgr/fdManager.cpp +++ b/modules/libcom/src/fdmgr/fdManager.cpp @@ -75,7 +75,7 @@ struct fdManagerPrivate { fdReg * pCBReg; fdManager & owner; - fdManagerPrivate(fdManager & owner); + explicit fdManagerPrivate(fdManager & owner); void lazyInitTimerQueue(); }; From 27f4261dfb0f98f87d8b9dad3d380d1d699dc736 Mon Sep 17 00:00:00 2001 From: Dirk Zimoch Date: Thu, 30 Jan 2025 12:02:49 +0100 Subject: [PATCH 09/15] use smart pointers in fdManager --- modules/libcom/src/fdmgr/fdManager.cpp | 9 ++++----- modules/libcom/src/fdmgr/fdManager.h | 12 +++++++++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/modules/libcom/src/fdmgr/fdManager.cpp b/modules/libcom/src/fdmgr/fdManager.cpp index 58917b4c7..8b8205f0f 100644 --- a/modules/libcom/src/fdmgr/fdManager.cpp +++ b/modules/libcom/src/fdmgr/fdManager.cpp @@ -56,7 +56,7 @@ struct fdManagerPrivate { tsDLList < fdReg > activeList; resTable < fdReg, fdRegId > fdTbl; const double sleepQuantum; - epicsTimerQueuePassive * pTimerQueue; + epics::auto_ptr pTimerQueue; bool processInProg; #ifdef FDMGR_USE_POLL @@ -81,14 +81,14 @@ struct fdManagerPrivate { fdManagerPrivate::fdManagerPrivate(fdManager & owner) : sleepQuantum(epicsThreadSleepQuantum()), - pTimerQueue(0), processInProg(false), + processInProg(false), pCBReg(0), owner(owner) {} inline void fdManagerPrivate::lazyInitTimerQueue () { - if (!pTimerQueue) { - pTimerQueue = & epicsTimerQueuePassive::create(owner); + if (!pTimerQueue.get()) { + pTimerQueue.reset(&epicsTimerQueuePassive::create(owner)); } } @@ -138,7 +138,6 @@ LIBCOM_API fdManager::~fdManager() pReg->state = fdReg::limbo; pReg->destroy(); } - delete priv->pTimerQueue; osiSockRelease(); } diff --git a/modules/libcom/src/fdmgr/fdManager.h b/modules/libcom/src/fdmgr/fdManager.h index 518539bc4..571dcc7a5 100644 --- a/modules/libcom/src/fdmgr/fdManager.h +++ b/modules/libcom/src/fdmgr/fdManager.h @@ -19,6 +19,16 @@ #ifndef fdManagerH_included #define fdManagerH_included +#include +namespace epics { +#if __cplusplus>=201103L +template +using auto_ptr = std::unique_ptr; +#else +using std::auto_ptr; +#endif +} + #include "libComAPI.h" // reset share lib defines #include "tsDLList.h" #include "resourceLib.h" @@ -87,7 +97,7 @@ public: epicsTimer & createTimer (); private: - struct fdManagerPrivate* priv; + epics::auto_ptr priv; void reschedule (); double quantum (); From c3f57ee81815fdeac94768e827a93eb98d666846 Mon Sep 17 00:00:00 2001 From: Dirk Zimoch Date: Thu, 30 Jan 2025 12:10:38 +0100 Subject: [PATCH 10/15] make fdManagerPrivate::fdReg volatile to avoid codacy warning --- modules/libcom/src/fdmgr/fdManager.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/modules/libcom/src/fdmgr/fdManager.cpp b/modules/libcom/src/fdmgr/fdManager.cpp index 8b8205f0f..8e218deb5 100644 --- a/modules/libcom/src/fdmgr/fdManager.cpp +++ b/modules/libcom/src/fdmgr/fdManager.cpp @@ -72,7 +72,7 @@ struct fdManagerPrivate { // Set to fdreg when in call back // and nill otherwise // - fdReg * pCBReg; + volatile fdReg * pCBReg; fdManager & owner; explicit fdManagerPrivate(fdManager & owner); @@ -82,7 +82,7 @@ struct fdManagerPrivate { fdManagerPrivate::fdManagerPrivate(fdManager & owner) : sleepQuantum(epicsThreadSleepQuantum()), processInProg(false), - pCBReg(0), owner(owner) + pCBReg(NULL), owner(owner) {} inline void fdManagerPrivate::lazyInitTimerQueue () @@ -236,8 +236,10 @@ LIBCOM_API void fdManager::process (double delay) while (priv->pollfds[i].fd != iter->getFD() || priv->pollfds[i].events != PollEvents[iter->getType()]) { + errlogPrintf("fdManager: skipping (removed?) pollfd %d (expected %d)\n", priv->pollfds[i].fd, iter->getFD()); i++; // skip pollfd of removed items if (i >= ioPending) { // skip unknown (inserted?) items + errlogPrintf("fdManager: skipping (inserted?) item %d\n", iter->getFD()); iter = tmp; tmp++; if (!iter.valid()) break; @@ -398,6 +400,7 @@ void fdManager::installReg (fdReg ®) if ( status != 0 ) { throwWithLocation ( fdInterestSubscriptionAlreadyExits () ); } +// errlogPrintf("fdManager::adding fd %d\n", reg.getFD()); } // @@ -442,6 +445,7 @@ void fdManager::removeReg (fdReg ®In) FD_CLR(regIn.getFD(), &priv->fdSets[regIn.getType()]); #endif +// errlogPrintf("fdManager::removing fd %d\n", regIn.getFD()); } // From c9183b5241ea63514d1b02524634ce2f07393db2 Mon Sep 17 00:00:00 2001 From: Dirk Zimoch Date: Thu, 30 Jan 2025 16:25:29 +0100 Subject: [PATCH 11/15] fdManager: filter poll flags for Window's sake --- modules/libcom/src/fdmgr/fdManager.cpp | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/modules/libcom/src/fdmgr/fdManager.cpp b/modules/libcom/src/fdmgr/fdManager.cpp index 8e218deb5..1056925d7 100644 --- a/modules/libcom/src/fdmgr/fdManager.cpp +++ b/modules/libcom/src/fdmgr/fdManager.cpp @@ -35,9 +35,7 @@ #ifdef FDMGR_USE_POLL #include -#if defined(_WIN32) -#define poll WSAPoll -#else +#if !defined(_WIN32) #include #endif @@ -45,6 +43,15 @@ static const short PollEvents[] = { // must match fdRegType POLLRDNORM | POLLRDBAND | POLLIN | POLLHUP | POLLERR, POLLWRBAND | POLLWRNORM | POLLOUT | POLLERR, POLLPRI}; + +#if defined(_WIN32) +#define poll WSAPoll +// Filter out PollEvents that Windows does not accept in events (only returns in revents) +#define WIN_POLLEVENT_FILTER(ev) static_cast((ev) & (POLLIN | POLLOUT)) +#else +// Linux, MacOS and RTEMS don't care +#define WIN_POLLEVENT_FILTER(ev) (ev) +#endif #endif #ifdef FDMGR_USE_SELECT @@ -101,7 +108,9 @@ epicsTimer & fdManager::createTimer() fdManager fileDescriptorManager; static const unsigned mSecPerSec = 1000u; +#ifdef FDMGR_USE_SELECT static const unsigned uSecPerSec = 1000u * mSecPerSec; +#endif // // fdManager::fdManager() @@ -179,11 +188,14 @@ LIBCOM_API void fdManager::process (double delay) #ifdef FDMGR_USE_POLL #if __cplusplus >= 201100L - priv->pollfds.emplace_back(pollfd{.fd = iter->getFD(), .events = PollEvents[iter->getType()]}); + priv->pollfds.emplace_back(pollfd{ + .fd = iter->getFD(), + .events = WIN_POLLEVENT_FILTER(PollEvents[iter->getType()]) + }); #else struct pollfd pollfd; pollfd.fd = iter->getFD(); - pollfd.events = PollEvents[iter->getType()]; + pollfd.events = WIN_POLLEVENT_FILTER(PollEvents[iter->getType()]); pollfd.revents = 0; priv->pollfds.push_back(pollfd); #endif @@ -234,7 +246,7 @@ LIBCOM_API void fdManager::process (double delay) // But just in case... int isave = i; while (priv->pollfds[i].fd != iter->getFD() || - priv->pollfds[i].events != PollEvents[iter->getType()]) + priv->pollfds[i].events != WIN_POLLEVENT_FILTER(PollEvents[iter->getType()])) { errlogPrintf("fdManager: skipping (removed?) pollfd %d (expected %d)\n", priv->pollfds[i].fd, iter->getFD()); i++; // skip pollfd of removed items From 8f1a3888c687c60acbe7896830fa278a9c1a8833 Mon Sep 17 00:00:00 2001 From: Dirk Zimoch Date: Fri, 31 Jan 2025 13:55:47 +0100 Subject: [PATCH 12/15] cleanup coding style in fdManager --- modules/libcom/src/fdmgr/fdManager.cpp | 131 +++++++++++++------------ modules/libcom/src/fdmgr/fdManager.h | 68 ++++++------- 2 files changed, 101 insertions(+), 98 deletions(-) diff --git a/modules/libcom/src/fdmgr/fdManager.cpp b/modules/libcom/src/fdmgr/fdManager.cpp index 1056925d7..9e133d2ad 100644 --- a/modules/libcom/src/fdmgr/fdManager.cpp +++ b/modules/libcom/src/fdmgr/fdManager.cpp @@ -42,7 +42,7 @@ static const short PollEvents[] = { // must match fdRegType POLLRDNORM | POLLRDBAND | POLLIN | POLLHUP | POLLERR, POLLWRBAND | POLLWRNORM | POLLOUT | POLLERR, - POLLPRI}; + POLLPRI }; #if defined(_WIN32) #define poll WSAPoll @@ -59,11 +59,11 @@ static const short PollEvents[] = { // must match fdRegType #endif struct fdManagerPrivate { - tsDLList < fdReg > regList; - tsDLList < fdReg > activeList; - resTable < fdReg, fdRegId > fdTbl; + tsDLList regList; + tsDLList activeList; + resTable fdTbl; const double sleepQuantum; - epics::auto_ptr pTimerQueue; + epics::auto_ptr pTimerQueue; bool processInProg; #ifdef FDMGR_USE_POLL @@ -79,27 +79,27 @@ struct fdManagerPrivate { // Set to fdreg when in call back // and nill otherwise // - volatile fdReg * pCBReg; - fdManager & owner; + volatile fdReg* pCBReg; + fdManager& owner; - explicit fdManagerPrivate(fdManager & owner); + explicit fdManagerPrivate(fdManager& owner); void lazyInitTimerQueue(); }; -fdManagerPrivate::fdManagerPrivate(fdManager & owner) : +fdManagerPrivate::fdManagerPrivate(fdManager& owner) : sleepQuantum(epicsThreadSleepQuantum()), processInProg(false), pCBReg(NULL), owner(owner) {} -inline void fdManagerPrivate::lazyInitTimerQueue () +inline void fdManagerPrivate::lazyInitTimerQueue() { if (!pTimerQueue.get()) { pTimerQueue.reset(&epicsTimerQueuePassive::create(owner)); } } -epicsTimer & fdManager::createTimer() +epicsTimer& fdManager::createTimer() { priv->lazyInitTimerQueue(); return priv->pTimerQueue->createTimer(); @@ -118,15 +118,15 @@ static const unsigned uSecPerSec = 1000u * mSecPerSec; // hopefully its a reasonable guess that poll()/select() and epicsThreadSleep() // will have the same sleep quantum // -LIBCOM_API fdManager::fdManager () : +LIBCOM_API fdManager::fdManager() : priv(new fdManagerPrivate(*this)) { - int status = osiSockAttach (); - assert (status); + int status = osiSockAttach(); + assert(status); #ifdef FDMGR_USE_SELECT priv->maxFD = 0; - for ( size_t i = 0u; i < fdrNEnums; i++ ) { + for (size_t i = 0u; i < fdrNEnums; i++) { FD_ZERO(&priv->fdSets[i]); } #endif @@ -137,7 +137,7 @@ LIBCOM_API fdManager::fdManager () : // LIBCOM_API fdManager::~fdManager() { - fdReg *pReg; + fdReg* pReg; while ((pReg = priv->regList.get())) { pReg->state = fdReg::limbo; @@ -153,9 +153,9 @@ LIBCOM_API fdManager::~fdManager() // // fdManager::process() // -LIBCOM_API void fdManager::process (double delay) +LIBCOM_API void fdManager::process(double delay) { - priv->lazyInitTimerQueue (); + priv->lazyInitTimerQueue(); // // no recursion @@ -173,7 +173,7 @@ LIBCOM_API void fdManager::process (double delay) // double minDelay = priv->pTimerQueue->process(epicsTime::getCurrent()); - if ( minDelay >= delay ) { + if (minDelay >= delay) { minDelay = delay; } @@ -183,7 +183,7 @@ LIBCOM_API void fdManager::process (double delay) int ioPending = 0; tsDLIter iter = priv->regList.firstIter(); - while ( iter.valid () ) { + while (iter.valid()) { ++ioPending; #ifdef FDMGR_USE_POLL @@ -207,20 +207,20 @@ LIBCOM_API void fdManager::process (double delay) ++iter; } - if ( ioPending ) { + if (ioPending) { #ifdef FDMGR_USE_POLL if (minDelay * mSecPerSec > INT_MAX) minDelay = INT_MAX / mSecPerSec; - int status = poll(&priv->pollfds.front(), // ancient C++ has no vector.data() + int status = poll(&priv->pollfds[0], // ancient C++ has no vector.data() ioPending, static_cast(minDelay * mSecPerSec)); int i = 0; #endif #ifdef FDMGR_USE_SELECT struct timeval tv; - tv.tv_sec = static_cast ( minDelay ); - tv.tv_usec = static_cast ( (minDelay-tv.tv_sec) * uSecPerSec ); + tv.tv_sec = static_cast(minDelay); + tv.tv_usec = static_cast((minDelay-tv.tv_sec) * uSecPerSec); int status = select(priv->maxFD, &priv->fdSets[fdrRead], @@ -230,14 +230,14 @@ LIBCOM_API void fdManager::process (double delay) priv->pTimerQueue->process(epicsTime::getCurrent()); - if ( status > 0 ) { + if (status > 0) { // // Look for activity // - iter = priv->regList.firstIter (); - while ( iter.valid () && status > 0 ) { - tsDLIter < fdReg > tmp = iter; + iter = priv->regList.firstIter(); + while (iter.valid() && status > 0) { + tsDLIter tmp = iter; tmp++; #ifdef FDMGR_USE_POLL @@ -279,7 +279,7 @@ LIBCOM_API void fdManager::process (double delay) // I am careful to prevent problems if they access the // above list while in a "callBack()" routine // - fdReg * pReg; + fdReg* pReg; while ((pReg = priv->activeList.get())) { pReg->state = fdReg::limbo; @@ -308,13 +308,13 @@ LIBCOM_API void fdManager::process (double delay) } } } - else if ( status < 0 ) { + else if (status < 0) { int errnoCpy = SOCKERRNO; #ifdef FDMGR_USE_SELECT // don't depend on flags being properly set if // an error is returned from select - for ( size_t i = 0u; i < fdrNEnums; i++ ) { + for (size_t i = 0u; i < fdrNEnums; i++) { FD_ZERO(&priv->fdSets[i]); } #endif @@ -322,10 +322,10 @@ LIBCOM_API void fdManager::process (double delay) // // print a message if it's an unexpected error // - if ( errnoCpy != SOCK_EINTR ) { + if (errnoCpy != SOCK_EINTR) { char sockErrBuf[64]; - epicsSocketConvertErrnoToString ( - sockErrBuf, sizeof ( sockErrBuf ) ); + epicsSocketConvertErrnoToString( + sockErrBuf, sizeof(sockErrBuf)); errlogPrintf("fdManager: " #ifdef FDMGR_USE_POLL "poll()" @@ -364,7 +364,7 @@ void fdReg::destroy() // fdReg::~fdReg() { - this->manager.removeReg(*this); + manager.removeReg(*this); } // @@ -372,31 +372,34 @@ fdReg::~fdReg() // void fdReg::show(unsigned level) const { - printf ("fdReg at %p\n", (void *) this); - if (level>1u) { - printf ("\tstate = %d, onceOnly = %d\n", - this->state, this->onceOnly); + printf("fdReg at %p\n", this); + if (level > 1u) { + printf("\tstate = %d, onceOnly = %d\n", + state, onceOnly); } - this->fdRegId::show(level); + fdRegId::show(level); } // // fdRegId::show() // -void fdRegId::show ( unsigned level ) const +void fdRegId::show(unsigned level) const { - printf ( "fdRegId at %p\n", - static_cast ( this ) ); - if ( level > 1u ) { - printf ( "\tfd = %d, type = %d\n", - int(this->fd), this->type ); + printf("fdRegId at %p\n", this); + if (level > 1u) { + printf("\tfd = %" +#if defined(_WIN32) + "I" +#endif + "d, type = %d\n", + fd, type); } } // -// fdManager::installReg () +// fdManager::installReg() // -void fdManager::installReg (fdReg ®) +void fdManager::installReg(fdReg ®) { #ifdef FDMGR_USE_SELECT priv->maxFD = std::max(priv->maxFD, reg.getFD()+1); @@ -409,21 +412,21 @@ void fdManager::installReg (fdReg ®) reg.state = fdReg::pending; int status = priv->fdTbl.add(reg); - if ( status != 0 ) { - throwWithLocation ( fdInterestSubscriptionAlreadyExits () ); + if (status != 0) { + throwWithLocation(fdInterestSubscriptionAlreadyExits()); } // errlogPrintf("fdManager::adding fd %d\n", reg.getFD()); } // -// fdManager::removeReg () +// fdManager::removeReg() // -void fdManager::removeReg (fdReg ®In) +void fdManager::removeReg(fdReg ®In) { - fdReg *pItemFound; + fdReg* pItemFound; pItemFound = priv->fdTbl.remove(regIn); - if (pItemFound!=®In) { + if (pItemFound != ®In) { errlogPrintf("fdManager::removeReg() bad fd registration object\n"); return; } @@ -461,15 +464,15 @@ void fdManager::removeReg (fdReg ®In) } // -// fdManager::reschedule () +// fdManager::reschedule() // NOOP - this only runs single threaded, and therefore they can only // add a new timer from places that will always end up in a reschedule // -void fdManager::reschedule () +void fdManager::reschedule() { } -double fdManager::quantum () +double fdManager::quantum() { return priv->sleepQuantum; } @@ -477,22 +480,22 @@ double fdManager::quantum () // // lookUpFD() // -LIBCOM_API fdReg *fdManager::lookUpFD (const SOCKET fd, const fdRegType type) +LIBCOM_API fdReg* fdManager::lookUpFD(const SOCKET fd, const fdRegType type) { - if (fd<0) { + if (fd < 0) { return NULL; } - fdRegId id (fd,type); + fdRegId id(fd,type); return priv->fdTbl.lookup(id); } // // fdReg::fdReg() // -fdReg::fdReg (const SOCKET fdIn, const fdRegType typIn, +fdReg::fdReg(const SOCKET fdIn, const fdRegType typIn, const bool onceOnlyIn, fdManager &managerIn) : - fdRegId (fdIn,typIn), state (limbo), - onceOnly (onceOnlyIn), manager (managerIn) + fdRegId(fdIn,typIn), state(limbo), + onceOnly(onceOnlyIn), manager(managerIn) { #ifdef FDMGR_USE_SELECT if (!FD_IN_FDSET(fdIn)) { @@ -501,7 +504,7 @@ fdReg::fdReg (const SOCKET fdIn, const fdRegType typIn, return; } #endif - this->manager.installReg (*this); + manager.installReg(*this); } template class resTable; diff --git a/modules/libcom/src/fdmgr/fdManager.h b/modules/libcom/src/fdmgr/fdManager.h index 571dcc7a5..e998131aa 100644 --- a/modules/libcom/src/fdmgr/fdManager.h +++ b/modules/libcom/src/fdmgr/fdManager.h @@ -47,27 +47,27 @@ class LIBCOM_API fdRegId { public: - fdRegId (const SOCKET fdIn, const fdRegType typeIn) : + fdRegId(const SOCKET fdIn, const fdRegType typeIn) : fd(fdIn), type(typeIn) {} - SOCKET getFD () const + SOCKET getFD() const { - return this->fd; + return fd; } - fdRegType getType () const + fdRegType getType() const { - return this->type; + return type; } - bool operator == (const fdRegId &idIn) const + bool operator == (const fdRegId& idIn) const { - return this->fd == idIn.fd && this->type==idIn.type; + return fd == idIn.fd && type == idIn.type; } - resTableIndex hash () const; + resTableIndex hash() const; - virtual void show (unsigned level) const; + virtual void show(unsigned level) const; virtual ~fdRegId() {} private: @@ -87,24 +87,24 @@ public: // class fdInterestSubscriptionAlreadyExits {}; - fdManager (); - virtual ~fdManager (); - void process ( double delay ); // delay parameter is in seconds + fdManager(); + virtual ~fdManager(); + void process(double delay); // delay parameter is in seconds // returns NULL if the fd is unknown - class fdReg *lookUpFD (const SOCKET fd, const fdRegType type); + class fdReg* lookUpFD(const SOCKET fd, const fdRegType type); - epicsTimer & createTimer (); + epicsTimer& createTimer(); private: epics::auto_ptr priv; - void reschedule (); - double quantum (); - void installReg (fdReg ®); - void removeReg (fdReg ®); - fdManager ( const fdManager & ); - fdManager & operator = ( const fdManager & ); + void reschedule(); + double quantum(); + void installReg(fdReg& reg); + void removeReg(fdReg& reg); + fdManager(const fdManager&); + fdManager& operator = (const fdManager&); friend class fdReg; }; @@ -124,11 +124,11 @@ class LIBCOM_API fdReg : public: - fdReg (const SOCKET fdIn, const fdRegType type, - const bool onceOnly=false, fdManager &manager = fileDescriptorManager); - virtual ~fdReg (); + fdReg(const SOCKET fdIn, const fdRegType type, + const bool onceOnly=false, fdManager& manager = fileDescriptorManager); + virtual ~fdReg(); - virtual void show (unsigned level) const; + virtual void show(unsigned level) const; // // Called by the file descriptor manager: @@ -139,7 +139,7 @@ public: // // fdReg::destroy() does a "delete this" // - virtual void destroy (); + virtual void destroy(); private: enum state {active, pending, limbo}; @@ -151,32 +151,32 @@ private: // lifetime of a fdReg object if the constructor // specified "onceOnly" // - virtual void callBack ()=0; + virtual void callBack() = 0; unsigned char state; // state enums go here unsigned char onceOnly; - fdManager &manager; + fdManager& manager; - fdReg ( const fdReg & ); - fdReg & operator = ( const fdReg & ); + fdReg(const fdReg&); + fdReg& operator = (const fdReg&); }; // // fdRegId::hash() // -inline resTableIndex fdRegId::hash () const +inline resTableIndex fdRegId::hash() const { const unsigned fdManagerHashTableMinIndexBits = 8; - const unsigned fdManagerHashTableMaxIndexBits = sizeof(SOCKET)*CHAR_BIT; + const unsigned fdManagerHashTableMaxIndexBits = sizeof(SOCKET) * CHAR_BIT; resTableIndex hashid; - hashid = integerHash ( fdManagerHashTableMinIndexBits, - fdManagerHashTableMaxIndexBits, this->fd ); + hashid = integerHash(fdManagerHashTableMinIndexBits, + fdManagerHashTableMaxIndexBits, fd); // // also evenly distribute based on the type of fdRegType // - hashid ^= this->type; + hashid ^= type; // // the result here is always masked to the From 9481deacb0d6556d3e746c894c39f9ee69081ce5 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 5 Feb 2025 09:08:53 -0800 Subject: [PATCH 13/15] fdManagerTest: quiet codacy warnings --- modules/libcom/test/fdManagerTest.cpp | 43 +++++++++++++++------------ 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/modules/libcom/test/fdManagerTest.cpp b/modules/libcom/test/fdManagerTest.cpp index 49c1e96ce..0d8751412 100644 --- a/modules/libcom/test/fdManagerTest.cpp +++ b/modules/libcom/test/fdManagerTest.cpp @@ -7,6 +7,8 @@ #include +#include + #include #include #include @@ -35,11 +37,13 @@ void set_non_blocking(SOCKET sd) // RAII for epicsTimer struct ScopedTimer { epicsTimer& timer; + explicit ScopedTimer(epicsTimer& t) :timer(t) {} ~ScopedTimer() { timer.destroy(); } }; struct ScopedFDReg { fdReg * const reg; + explicit ScopedFDReg(fdReg* reg) :reg(reg) {} ~ScopedFDReg() { reg->destroy(); } }; @@ -67,19 +71,19 @@ public: void swap(Socket& o) { std::swap(sd, o.sd); } - sockaddr_in bind() + osiSockAddr bind() { - sockaddr_in addr; + osiSockAddr addr; memset(&addr, 0, sizeof(addr)); - addr.sin_family = AF_INET; - addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + addr.ia.sin_family = AF_INET; + addr.ia.sin_addr.s_addr = htonl(INADDR_LOOPBACK); - if(::bind(sd, (sockaddr*)&addr, sizeof(addr))) + if(::bind(sd, &addr.sa, sizeof(addr))) testAbort("Unable to bind lo : %d", SOCKERRNO); - sockaddr_in ret; + osiSockAddr ret; osiSocklen_t addrlen = sizeof(ret); - if(getsockname(sd, (sockaddr*)&ret, &addrlen)) + if(getsockname(sd, &ret.sa, &addrlen)) testAbort("Unable to getsockname : %d", SOCKERRNO); (void)addrlen; @@ -92,14 +96,14 @@ public: struct DoConnect final : public epicsThreadRunable { const SOCKET sd; - sockaddr_in to; - DoConnect(SOCKET sd, const sockaddr_in& to) + osiSockAddr to; + DoConnect(SOCKET sd, const osiSockAddr& to) :sd(sd) ,to(to) {} void run() override final { - int err = connect(sd, (sockaddr*)&to, sizeof(to)); + int err = connect(sd, &to.sa, sizeof(to)); testOk(err==0, "connect() %d %d", err, SOCKERRNO); } }; @@ -107,11 +111,12 @@ struct DoConnect final : public epicsThreadRunable { struct DoAccept final : public epicsThreadRunable { const SOCKET sd; Socket peer; - sockaddr_in peer_addr; + osiSockAddr peer_addr; + explicit DoAccept(SOCKET sd) :sd(sd) {} void run() override final { osiSocklen_t len(sizeof(peer_addr)); - Socket temp(accept(sd, (sockaddr*)&peer_addr, &len)); + Socket temp(accept(sd, &peer_addr.sa, &len)); if(temp.sd==INVALID_SOCKET) testFail("accept() -> %d", SOCKERRNO); temp.swap(peer); @@ -120,12 +125,12 @@ struct DoAccept final : public epicsThreadRunable { struct DoRead final : public epicsThreadRunable { const SOCKET sd; - void* buf; + char* buf; unsigned buflen; int n; - DoRead(SOCKET sd, void* buf, unsigned buflen): sd(sd), buf(buf), buflen(buflen), n(0) {} + DoRead(SOCKET sd, char* buf, unsigned buflen): sd(sd), buf(buf), buflen(buflen), n(0) {} void run() override final { - n = recv(sd, (char*)buf, buflen, 0); + n = recv(sd, buf, buflen, 0); if(n<0) testFail("read() -> %d, %d", n, SOCKERRNO); } @@ -133,13 +138,13 @@ struct DoRead final : public epicsThreadRunable { struct DoWriteAll final : public epicsThreadRunable { const SOCKET sd; - const void* buf; + const char* buf; unsigned buflen; - DoWriteAll(SOCKET sd, const void* buf, unsigned buflen): sd(sd), buf(buf), buflen(buflen) {} + DoWriteAll(SOCKET sd, const char* buf, unsigned buflen): sd(sd), buf(buf), buflen(buflen) {} void run() override final { unsigned nsent = 0; while(nsent %d, %d", n, SOCKERRNO); break; @@ -222,7 +227,7 @@ void testSockIO() fdManager mgr; Socket listener(AF_INET, SOCK_STREAM); set_non_blocking(listener.sd); - sockaddr_in servAddr(listener.bind()); + osiSockAddr servAddr(listener.bind()); Socket client(AF_INET, SOCK_STREAM); Socket server; From 9c1334ae15b65b7150903dc6f33dd31e7e13e41b Mon Sep 17 00:00:00 2001 From: Dirk Zimoch Date: Thu, 6 Feb 2025 14:09:20 +0100 Subject: [PATCH 14/15] silence Codacy warning --- modules/libcom/test/fdManagerTest.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/libcom/test/fdManagerTest.cpp b/modules/libcom/test/fdManagerTest.cpp index 0d8751412..0dc1b330c 100644 --- a/modules/libcom/test/fdManagerTest.cpp +++ b/modules/libcom/test/fdManagerTest.cpp @@ -73,8 +73,7 @@ public: } osiSockAddr bind() { - osiSockAddr addr; - memset(&addr, 0, sizeof(addr)); + osiSockAddr addr = {0}; addr.ia.sin_family = AF_INET; addr.ia.sin_addr.s_addr = htonl(INADDR_LOOPBACK); From bc27476554073e2e6da202215c3bd32e54985bed Mon Sep 17 00:00:00 2001 From: Dirk Zimoch Date: Thu, 6 Feb 2025 17:45:20 +0100 Subject: [PATCH 15/15] document fdManager change --- documentation/RELEASE_NOTES.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/documentation/RELEASE_NOTES.md b/documentation/RELEASE_NOTES.md index ce4088385..c13489892 100644 --- a/documentation/RELEASE_NOTES.md +++ b/documentation/RELEASE_NOTES.md @@ -22,6 +22,12 @@ should also be read to understand what has changed since earlier releases: ## Changes made on the 7.0 branch since 7.0.8.1 +### fdManager file descriptor limit removed + +In order to support file descriptors above 1023, fdManager now uses +poll() instead of select() on all architectures that support it +(Linux, MacOS, Windows, newer RTEMS). + ### Post monitors from compress record when it's reset Writing into a compress record's `RES` field now posts a monitor event instead