diff --git a/documentation/RELEASE_NOTES.html b/documentation/RELEASE_NOTES.html
index 859ff7b29..b2465e53c 100644
--- a/documentation/RELEASE_NOTES.html
+++ b/documentation/RELEASE_NOTES.html
@@ -13,6 +13,12 @@
+Fix DNS related crash on exit
+
+The attempt to fix DNS related delays for short lived CLI programs (eg. caget)
+in lp:1527636 introduced a bug which cased these short lived clients to crash on exit.
+This bug should now be fixed.
+
Server bind issue on Windows
When a National Instruments network variables CA server is already running on
diff --git a/src/libCom/misc/ipAddrToAsciiAsynchronous.cpp b/src/libCom/misc/ipAddrToAsciiAsynchronous.cpp
index 6dbdc405e..c6fea15b4 100644
--- a/src/libCom/misc/ipAddrToAsciiAsynchronous.cpp
+++ b/src/libCom/misc/ipAddrToAsciiAsynchronous.cpp
@@ -18,6 +18,9 @@
#include
#include
+//#define EPICS_FREELIST_DEBUG
+#define EPICS_PRIVATE_API
+
#define epicsExportSharedSymbols
#include "ipAddrToAsciiAsynchronous.h"
#include "epicsThread.h"
@@ -45,7 +48,6 @@ public:
< ipAddrToAsciiTransactionPrivate, 0x80 > & );
epicsPlacementDeleteOperator (( void *, tsFreeList
< ipAddrToAsciiTransactionPrivate, 0x80 > & ))
-private:
osiSockAddr addr;
ipAddrToAsciiEnginePrivate & engine;
ipAddrToAsciiCallBack * pCB;
@@ -54,7 +56,7 @@ private:
void release ();
void * operator new ( size_t );
void operator delete ( void * );
- friend class ipAddrToAsciiEnginePrivate;
+private:
ipAddrToAsciiTransactionPrivate & operator = ( const ipAddrToAsciiTransactionPrivate & );
ipAddrToAsciiTransactionPrivate ( const ipAddrToAsciiTransactionPrivate & );
};
@@ -75,41 +77,54 @@ extern "C" {
static void ipAddrToAsciiEngineGlobalMutexConstruct ( void * );
}
-// - this class executes the synchronous DNS query
-// - it creates one thread
-class ipAddrToAsciiEnginePrivate :
- public ipAddrToAsciiEngine,
- public epicsThreadRunable {
-public:
- ipAddrToAsciiEnginePrivate ();
- virtual ~ipAddrToAsciiEnginePrivate ();
- void show ( unsigned level ) const;
-private:
+namespace {
+struct ipAddrToAsciiGlobal : public epicsThreadRunable {
+ ipAddrToAsciiGlobal();
+ virtual ~ipAddrToAsciiGlobal() {}
+
+ virtual void run ();
+
char nameTmp [1024];
- tsFreeList
- < ipAddrToAsciiTransactionPrivate, 0x80 >
+ tsFreeList
+ < ipAddrToAsciiTransactionPrivate, 0x80 >
transactionFreeList;
tsDLList < ipAddrToAsciiTransactionPrivate > labor;
mutable epicsMutex mutex;
epicsEvent laborEvent;
epicsEvent destructorBlockEvent;
epicsThread thread;
+ // pCurrent may be changed by any thread (worker or other)
ipAddrToAsciiTransactionPrivate * pCurrent;
+ // pActive may only be changed by the worker
+ ipAddrToAsciiTransactionPrivate * pActive;
unsigned cancelPendingCount;
bool exitFlag;
bool callbackInProgress;
- static ipAddrToAsciiEnginePrivate * pEngine;
+};
+}
+
+// - this class executes the synchronous DNS query
+// - it creates one thread
+class ipAddrToAsciiEnginePrivate :
+ public ipAddrToAsciiEngine {
+public:
+ ipAddrToAsciiEnginePrivate() :refcount(1u), released(false) {}
+ virtual ~ipAddrToAsciiEnginePrivate () {}
+ void show ( unsigned level ) const;
+
+ unsigned refcount;
+ bool released;
+
+ static ipAddrToAsciiGlobal * pEngine;
ipAddrToAsciiTransaction & createTransaction ();
- void release ();
- void run ();
- ipAddrToAsciiEnginePrivate ( const ipAddrToAsciiEngine & );
+ void release ();
+
+private:
+ ipAddrToAsciiEnginePrivate ( const ipAddrToAsciiEngine & );
ipAddrToAsciiEnginePrivate & operator = ( const ipAddrToAsciiEngine & );
- friend class ipAddrToAsciiEngine;
- friend class ipAddrToAsciiTransactionPrivate;
- friend void ipAddrToAsciiEngineGlobalMutexConstruct ( void * );
};
-ipAddrToAsciiEnginePrivate * ipAddrToAsciiEnginePrivate :: pEngine = 0;
+ipAddrToAsciiGlobal * ipAddrToAsciiEnginePrivate :: pEngine = 0;
static epicsThreadOnceId ipAddrToAsciiEngineGlobalMutexOnceFlag = EPICS_THREAD_ONCE_INIT;
// the users are not required to supply a show routine
@@ -124,12 +139,24 @@ ipAddrToAsciiEngine::~ipAddrToAsciiEngine () {}
static void ipAddrToAsciiEngineGlobalMutexConstruct ( void * )
{
try{
- ipAddrToAsciiEnginePrivate::pEngine = new ipAddrToAsciiEnginePrivate ();
+ ipAddrToAsciiEnginePrivate::pEngine = new ipAddrToAsciiGlobal ();
}catch(std::exception& e){
errlogPrintf("ipAddrToAsciiEnginePrivate ctor fails with: %s\n", e.what());
}
}
+void ipAddrToAsciiEngine::cleanup()
+{
+ {
+ epicsGuard G(ipAddrToAsciiEnginePrivate::pEngine->mutex);
+ ipAddrToAsciiEnginePrivate::pEngine->exitFlag = true;
+ }
+ ipAddrToAsciiEnginePrivate::pEngine->laborEvent.signal();
+ ipAddrToAsciiEnginePrivate::pEngine->thread.exitWait();
+ delete ipAddrToAsciiEnginePrivate::pEngine;
+ ipAddrToAsciiEnginePrivate::pEngine = 0;
+}
+
// for now its probably sufficent to allocate one
// DNS transaction thread for all codes sharing
// the same process that need DNS services but we
@@ -141,41 +168,78 @@ ipAddrToAsciiEngine & ipAddrToAsciiEngine::allocate ()
ipAddrToAsciiEngineGlobalMutexConstruct, 0 );
if(!ipAddrToAsciiEnginePrivate::pEngine)
throw std::runtime_error("ipAddrToAsciiEngine::allocate fails");
- return * ipAddrToAsciiEnginePrivate::pEngine;
+ return * new ipAddrToAsciiEnginePrivate();
}
-ipAddrToAsciiEnginePrivate::ipAddrToAsciiEnginePrivate () :
+ipAddrToAsciiGlobal::ipAddrToAsciiGlobal () :
thread ( *this, "ipToAsciiProxy",
epicsThreadGetStackSize(epicsThreadStackBig),
epicsThreadPriorityLow ),
- pCurrent ( 0 ), cancelPendingCount ( 0u ), exitFlag ( false ),
+ pCurrent ( 0 ), pActive ( 0 ), cancelPendingCount ( 0u ), exitFlag ( false ),
callbackInProgress ( false )
{
this->thread.start (); // start the thread
}
-ipAddrToAsciiEnginePrivate::~ipAddrToAsciiEnginePrivate ()
-{
- {
- epicsGuard < epicsMutex > guard ( this->mutex );
- this->exitFlag = true;
- }
- this->laborEvent.signal ();
- this->thread.exitWait ();
-}
void ipAddrToAsciiEnginePrivate::release ()
{
+ bool last;
+ {
+ epicsGuard < epicsMutex > guard ( this->pEngine->mutex );
+ if(released)
+ throw std::logic_error("Engine release() called again!");
+
+ // released==true prevents new transactions
+ released = true;
+
+ {
+ // cancel any pending transactions
+ tsDLIter < ipAddrToAsciiTransactionPrivate > it(pEngine->labor.firstIter());
+ while(it.valid()) {
+ ipAddrToAsciiTransactionPrivate *trn = it.pointer();
+ ++it;
+
+ if(this==&trn->engine) {
+ trn->pending = false;
+ pEngine->labor.remove(*trn);
+ }
+ }
+
+ // cancel transaction in lookup or callback
+ if (pEngine->pCurrent && this==&pEngine->pCurrent->engine) {
+ pEngine->pCurrent->pending = false;
+ pEngine->pCurrent = 0;
+ }
+
+ // wait for completion of in-progress callback
+ pEngine->cancelPendingCount++;
+ while(pEngine->pActive && this==&pEngine->pActive->engine
+ && ! pEngine->thread.isCurrentThread()) {
+ epicsGuardRelease < epicsMutex > unguard ( guard );
+ pEngine->destructorBlockEvent.wait();
+ }
+ pEngine->cancelPendingCount--;
+ if(pEngine->cancelPendingCount)
+ pEngine->destructorBlockEvent.signal();
+ }
+
+ assert(refcount>0);
+ last = 0==--refcount;
+ }
+ if(last) {
+ delete this;
+ }
}
void ipAddrToAsciiEnginePrivate::show ( unsigned level ) const
{
- epicsGuard < epicsMutex > guard ( this->mutex );
+ epicsGuard < epicsMutex > guard ( this->pEngine->mutex );
printf ( "ipAddrToAsciiEngine at %p with %u requests pending\n",
- static_cast (this), this->labor.count () );
+ static_cast (this), this->pEngine->labor.count () );
if ( level > 0u ) {
- tsDLIterConst < ipAddrToAsciiTransactionPrivate >
- pItem = this->labor.firstIter ();
+ tsDLIter < ipAddrToAsciiTransactionPrivate >
+ pItem = this->pEngine->labor.firstIter ();
while ( pItem.valid () ) {
pItem->show ( level - 1u );
pItem++;
@@ -183,10 +247,10 @@ void ipAddrToAsciiEnginePrivate::show ( unsigned level ) const
}
if ( level > 1u ) {
printf ( "mutex:\n" );
- this->mutex.show ( level - 2u );
+ this->pEngine->mutex.show ( level - 2u );
printf ( "laborEvent:\n" );
- this->laborEvent.show ( level - 2u );
- printf ( "exitFlag boolean = %u\n", this->exitFlag );
+ this->pEngine->laborEvent.show ( level - 2u );
+ printf ( "exitFlag boolean = %u\n", this->pEngine->exitFlag );
printf ( "exit event:\n" );
}
}
@@ -226,10 +290,20 @@ void ipAddrToAsciiTransactionPrivate::operator delete ( void * )
ipAddrToAsciiTransaction & ipAddrToAsciiEnginePrivate::createTransaction ()
{
- return * new ( this->transactionFreeList ) ipAddrToAsciiTransactionPrivate ( *this );
+ epicsGuard G(this->pEngine->mutex);
+ if(this->released)
+ throw std::logic_error("createTransaction() on release()'d ipAddrToAsciiEngine");
+
+ assert(this->refcount>0);
+
+ ipAddrToAsciiTransactionPrivate *ret = new ( this->pEngine->transactionFreeList ) ipAddrToAsciiTransactionPrivate ( *this );
+
+ this->refcount++;
+
+ return * ret;
}
-void ipAddrToAsciiEnginePrivate::run ()
+void ipAddrToAsciiGlobal::run ()
{
epicsGuard < epicsMutex > guard ( this->mutex );
while ( ! this->exitFlag ) {
@@ -267,7 +341,7 @@ void ipAddrToAsciiEnginePrivate::run ()
// fix for lp:1580623
// a destructing cac sets pCurrent to NULL, so
// make local copy to avoid race when releasing the guard
- ipAddrToAsciiTransactionPrivate *pCur = this->pCurrent;
+ ipAddrToAsciiTransactionPrivate *pCur = pActive = pCurrent;
this->callbackInProgress = true;
{
@@ -277,6 +351,7 @@ void ipAddrToAsciiEnginePrivate::run ()
}
this->callbackInProgress = false;
+ pActive = 0;
if ( this->pCurrent ) {
this->pCurrent->pending = false;
@@ -300,44 +375,53 @@ ipAddrToAsciiTransactionPrivate::ipAddrToAsciiTransactionPrivate
void ipAddrToAsciiTransactionPrivate::release ()
{
this->~ipAddrToAsciiTransactionPrivate ();
- this->engine.transactionFreeList.release ( this );
+ this->engine.pEngine->transactionFreeList.release ( this );
}
ipAddrToAsciiTransactionPrivate::~ipAddrToAsciiTransactionPrivate ()
{
- epicsGuard < epicsMutex > guard ( this->engine.mutex );
- while ( this->pending ) {
- if ( this->engine.pCurrent == this &&
- this->engine.callbackInProgress &&
- ! this->engine.thread.isCurrentThread() ) {
- // cancel from another thread while callback in progress
- // waits for callback to complete
- assert ( this->engine.cancelPendingCount < UINT_MAX );
- this->engine.cancelPendingCount++;
- {
- epicsGuardRelease < epicsMutex > unguard ( guard );
- this->engine.destructorBlockEvent.wait ();
- }
- assert ( this->engine.cancelPendingCount > 0u );
- this->engine.cancelPendingCount--;
- if ( ! this->pending ) {
- if ( this->engine.cancelPendingCount ) {
- this->engine.destructorBlockEvent.signal ();
+ ipAddrToAsciiGlobal *pGlobal = this->engine.pEngine;
+ bool last;
+ {
+ epicsGuard < epicsMutex > guard ( pGlobal->mutex );
+ while ( this->pending ) {
+ if ( pGlobal->pCurrent == this &&
+ pGlobal->callbackInProgress &&
+ ! pGlobal->thread.isCurrentThread() ) {
+ // cancel from another thread while callback in progress
+ // waits for callback to complete
+ assert ( pGlobal->cancelPendingCount < UINT_MAX );
+ pGlobal->cancelPendingCount++;
+ {
+ epicsGuardRelease < epicsMutex > unguard ( guard );
+ pGlobal->destructorBlockEvent.wait ();
+ }
+ assert ( pGlobal->cancelPendingCount > 0u );
+ pGlobal->cancelPendingCount--;
+ if ( ! this->pending ) {
+ if ( pGlobal->cancelPendingCount ) {
+ pGlobal->destructorBlockEvent.signal ();
+ }
+ break;
}
- break;
- }
- }
- else {
- if ( this->engine.pCurrent == this ) {
- // cancel from callback, or while lookup in progress
- this->engine.pCurrent = 0;
}
else {
- // cancel before lookup starts
- this->engine.labor.remove ( *this );
+ if ( pGlobal->pCurrent == this ) {
+ // cancel from callback, or while lookup in progress
+ pGlobal->pCurrent = 0;
+ }
+ else {
+ // cancel before lookup starts
+ pGlobal->labor.remove ( *this );
+ }
+ this->pending = false;
}
- this->pending = false;
}
+ assert(this->engine.refcount>0);
+ last = 0==--this->engine.refcount;
+ }
+ if(last) {
+ delete &this->engine;
}
}
@@ -345,15 +429,21 @@ void ipAddrToAsciiTransactionPrivate::ipAddrToAscii (
const osiSockAddr & addrIn, ipAddrToAsciiCallBack & cbIn )
{
bool success;
+ ipAddrToAsciiGlobal *pGlobal = this->engine.pEngine;
{
- epicsGuard < epicsMutex > guard ( this->engine.mutex );
- // put some reasonable limit on queue expansion
- if ( !this->pending && engine.labor.count () < 16u ) {
+ epicsGuard < epicsMutex > guard ( pGlobal->mutex );
+
+ if (this->engine.released) {
+ errlogPrintf("Warning: ipAddrToAscii on transaction with release()'d ipAddrToAsciiEngine");
+ success = false;
+
+ } else if ( !this->pending && pGlobal->labor.count () < 16u ) {
+ // put some reasonable limit on queue expansion
this->addr = addrIn;
this->pCB = & cbIn;
this->pending = true;
- this->engine.labor.add ( *this );
+ pGlobal->labor.add ( *this );
success = true;
}
else {
@@ -362,7 +452,7 @@ void ipAddrToAsciiTransactionPrivate::ipAddrToAscii (
}
if ( success ) {
- this->engine.laborEvent.signal ();
+ pGlobal->laborEvent.signal ();
}
else {
char autoNameTmp[256];
@@ -379,7 +469,7 @@ osiSockAddr ipAddrToAsciiTransactionPrivate::address () const
void ipAddrToAsciiTransactionPrivate::show ( unsigned level ) const
{
- epicsGuard < epicsMutex > guard ( this->engine.mutex );
+ epicsGuard < epicsMutex > guard ( this->engine.pEngine->mutex );
char ipAddr [64];
sockAddrToDottedIP ( &this->addr.sa, ipAddr, sizeof ( ipAddr ) );
printf ( "ipAddrToAsciiTransactionPrivate for address %s\n", ipAddr );
diff --git a/src/libCom/misc/ipAddrToAsciiAsynchronous.h b/src/libCom/misc/ipAddrToAsciiAsynchronous.h
index f2de879b6..5da06860a 100644
--- a/src/libCom/misc/ipAddrToAsciiAsynchronous.h
+++ b/src/libCom/misc/ipAddrToAsciiAsynchronous.h
@@ -44,6 +44,10 @@ public:
static ipAddrToAsciiEngine & allocate ();
protected:
virtual ~ipAddrToAsciiEngine () = 0;
+public:
+#ifdef EPICS_PRIVATE_API
+ static void cleanup();
+#endif
};
#endif // ifdef ipAddrToAsciiAsynchronous_h
diff --git a/src/libCom/test/Makefile b/src/libCom/test/Makefile
index 5f486938f..6239e0b41 100644
--- a/src/libCom/test/Makefile
+++ b/src/libCom/test/Makefile
@@ -11,6 +11,7 @@ TOP=../../..
include $(TOP)/configure/CONFIG
PROD_LIBS += Com
+PROD_SYS_LIBS_WIN32 += ws2_32 advapi32 user32
TESTPROD_HOST += epicsUnitTestTest
epicsUnitTestTest_SRCS += epicsUnitTestTest.c
@@ -151,6 +152,10 @@ epicsMessageQueueTest_SRCS += epicsMessageQueueTest.cpp
testHarness_SRCS += epicsMessageQueueTest.cpp
TESTS += epicsMessageQueueTest
+TESTPROD_HOST += ipAddrToAsciiTest
+ipAddrToAsciiTest_SRCS += ipAddrToAsciiTest.cpp
+testHarness_SRCS += ipAddrToAsciiTest.cpp
+TESTS += ipAddrToAsciiTest
# The testHarness runs all the test programs in a known working order.
testHarness_SRCS += epicsRunLibComTests.c
diff --git a/src/libCom/test/epicsRunLibComTests.c b/src/libCom/test/epicsRunLibComTests.c
index 81290644a..16d316592 100644
--- a/src/libCom/test/epicsRunLibComTests.c
+++ b/src/libCom/test/epicsRunLibComTests.c
@@ -41,6 +41,7 @@ int macEnvExpandTest(void);
int ringPointerTest(void);
int ringBytesTest(void);
int blockingSockTest(void);
+int ipAddrToAsciiTest(void);
int epicsSockResolveTest(void);
int taskwdTest(void);
int epicsExitTest(void);
@@ -102,6 +103,8 @@ void epicsRunLibComTests(void)
runTest(ringBytesTest);
runTest(blockingSockTest);
+
+ runTest(ipAddrToAsciiTest);
runTest(epicsSockResolveTest);
diff --git a/src/libCom/test/ipAddrToAsciiTest.cpp b/src/libCom/test/ipAddrToAsciiTest.cpp
new file mode 100644
index 000000000..f9323374d
--- /dev/null
+++ b/src/libCom/test/ipAddrToAsciiTest.cpp
@@ -0,0 +1,165 @@
+/*************************************************************************\
+* Copyright (c) 2017 Michael Davidsaver
+* EPICS BASE is distributed subject to a Software License Agreement found
+* in file LICENSE that is included with this distribution.
+\*************************************************************************/
+
+#include
+#include
+
+#define EPICS_PRIVATE_API
+
+#include "epicsMutex.h"
+#include "epicsGuard.h"
+#include "epicsThread.h"
+#include "epicsEvent.h"
+#include "ipAddrToAsciiAsynchronous.h"
+
+#include "epicsUnitTest.h"
+#include "testMain.h"
+
+namespace {
+
+typedef epicsGuard Guard;
+typedef epicsGuardRelease UnGuard;
+
+struct CB : public ipAddrToAsciiCallBack
+{
+ const char *name;
+ epicsMutex mutex;
+ epicsEvent starter, blocker, complete;
+ bool started, cont, done;
+ CB(const char *name) : name(name), started(false), cont(false), done(false) {}
+ virtual ~CB() {}
+ virtual void transactionComplete ( const char * pHostName )
+ {
+ Guard G(mutex);
+ started = true;
+ starter.signal();
+ testDiag("In transactionComplete(%s) for %s", pHostName, name);
+ while(!cont) {
+ UnGuard U(G);
+ if(!blocker.wait(2.0))
+ break;
+ }
+ done = true;
+ complete.signal();
+ }
+ void waitStart()
+ {
+ Guard G(mutex);
+ while(!started) {
+ UnGuard U(G);
+ if(!starter.wait(2.0))
+ break;
+ }
+ }
+ void poke()
+ {
+ testDiag("Poke");
+ Guard G(mutex);
+ cont = true;
+ blocker.signal();
+ }
+ void finish()
+ {
+ testDiag("Finish");
+ Guard G(mutex);
+ while(!done) {
+ UnGuard U(G);
+ if(!complete.wait(2.0))
+ break;
+ }
+ testDiag("Finished");
+ }
+};
+
+// ensure that lookup of 127.0.0.1 works
+void doLookup(ipAddrToAsciiEngine& engine)
+{
+ testDiag("In doLookup");
+
+ ipAddrToAsciiTransaction& trn(engine.createTransaction());
+ CB cb("cb");
+ osiSockAddr addr;
+ addr.ia.sin_family = AF_INET;
+ addr.ia.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+ addr.ia.sin_port = htons(42);
+
+ testDiag("Start lookup");
+ trn.ipAddrToAscii(addr, cb);
+ cb.poke();
+ cb.finish();
+ testOk1(cb.cont);
+ testOk1(cb.done);
+
+ trn.release();
+}
+
+// Test cancel of pending transaction
+void doCancel()
+{
+ testDiag("In doCancel");
+
+ ipAddrToAsciiEngine& engine1(ipAddrToAsciiEngine::allocate());
+ ipAddrToAsciiEngine& engine2(ipAddrToAsciiEngine::allocate());
+
+ ipAddrToAsciiTransaction& trn1(engine1.createTransaction()),
+ & trn2(engine2.createTransaction());
+ testOk1(&trn1!=&trn2);
+ CB cb1("cb1"), cb2("cb2");
+
+ osiSockAddr addr;
+ addr.ia.sin_family = AF_INET;
+ addr.ia.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+ addr.ia.sin_port = htons(42);
+
+ // ensure that the worker thread is blocked with a transaction from engine1
+ testDiag("Start lookup1");
+ trn1.ipAddrToAscii(addr, cb1);
+ testDiag("Wait start1");
+ cb1.waitStart();
+
+ testDiag("Start lookup2");
+ trn2.ipAddrToAscii(addr, cb2);
+
+ testDiag("release engine2, implicitly cancels lookup2");
+ engine2.release();
+
+ cb2.poke();
+ testDiag("Wait for lookup2 timeout");
+ cb2.finish();
+ testOk1(!cb2.done);
+
+ testDiag("Complete lookup1");
+ cb1.poke();
+ cb1.finish();
+ testOk1(cb1.done);
+
+ engine1.release();
+
+ trn1.release();
+ trn2.release();
+}
+
+} // namespace
+
+MAIN(ipAddrToAsciiTest)
+{
+ testPlan(5);
+ {
+ ipAddrToAsciiEngine& engine(ipAddrToAsciiEngine::allocate());
+ doLookup(engine);
+ engine.release();
+ }
+ doCancel();
+ // TODO: somehow test cancel of in-progress callback
+ // allow time for any un-canceled transcations to crash us...
+ epicsThreadSleep(1.0);
+
+#ifdef __linux__
+ ipAddrToAsciiEngine::cleanup();
+#endif
+
+ return testDone();
+}