Merge lp:~epics-core/epics-base/fix-async-dns into lp:~epics-core/epics-base/3.14

[mdavidsaver]

An attempt to properly address the regressions exposed/uncovered by lp:1527636. Previous to that change the ipAddrToAscii worker thread was stopped by the last call to ipAddrToAsciiEngine::release(), preventing further transactions from executing. The change (by myself) for lp:1527636 made release() a no-op, which did not stop pending transactions. This lead to races involving msgForMultiply, and perhaps others.

The original code was still open to races when more than one call to ipAddrToAsciiEngine::allocate() was made. The shared singleton ipAddrToAsciiEngine would not stop the worker until the last release(). So the race with msgForMultiply could happen if, for example, more than one CA context exists.

This branch attempt to achieve the best of both worlds. The worker thread is not stopped, but the semantics of release() are restored, and improved. The semantics of the API should be otherwise unchanged.

Instead of being a singleton ipAddrToAsciiEngine becomes an empty struct to trace references. The singleton is renamed ipAddrToAsciiGlobal and is fully hidden from user code.

Each all to ipAddrToAsciiEngine::allocate() returns a new, unique, ipAddrToAsciiEngine instance. release() must be called to "destroy". ipAddrToAsciiEnginePrivate carries a reference counter to ensure it outlives any transactions which reference it.

Calling ipAddrToAsciiEngine::release() marks the engine as "released" which will any transactions from being pended. It cancels and already pending transactions with engine==this and wait()s if the currently executing callback is the same. Canceled transactions are not executed.

[Andrew Johnson (anj)]

Subject to merge testing, accepted in principle at 3/14/2017 F2F meeting.

[Ralph Lange (ralph-lange)]

Code looks good to me.

I am not able to create an original setup that allows reproducing the error.
However, running the added test against an unpatched base I see occasional failures (test 4 fails in about 20% of the cases), while running it against this branch does not fail.

review: Approve

[Andrew Johnson (anj)]

Is there any reason to not run the new ipAddrToAsciiTest code on RTEMS and/or VxWorks? It has not been added to epicsRunLibComTests.c so it isn't included in the test harness at the moment.

[mdavidsaver (mdavidsaver)]

No reason, an oversight on my part. I'll add this evening if you haven't already.

[mdavidsaver (mdavidsaver)]

Done w/ the git branch. I'm going to try to see if I can switch this MR to the git branches w/o restarting it. Be prepared for some email...

[mdavidsaver (mdavidsaver)]

FYI, "Resubmit proposal" errors when trying to switch to git branches. I'm not going to manually start a new MR unless a larger change is needed.

[Andrew Johnson (anj)]

This passes its self-test fine on VxWorks.

However a second run inside the harness (without rebooting first) caused this:

***** ipAddrToAsciiTest *****
1..5
0x475798 (tShell0): Unhandled C++ exception resulted in call to terminate

0x00548424 epicsRunLibComTests+0x250: runTestFunc ()
0x00559584 runTestFunc +0x4c : ipAddrToAsciiTest ()
0x00547500 ipAddrToAsciiTest+0x7c : 0x00557b90 ()
0x00557b90 ipAddrToAsciiEngine::allocate()+0x47c: __cxa_throw ()
0x0027c578 __cxa_throw +0x70 : std::terminate() ()
0x00279e64 __cxxabiv1::__unexpected(void (*)())+0x0 : __cxxabiv1::__terminate(void (*)()) ()
0x00279e30 __cxxabiv1::__terminate(void (*)())+0x18 : cplusTerminate ()
0x00133c70 cplusTerminate+0x4c : taskSuspend ()

Is the ipAddrToAsciiEngine::cleanup() routine necessary (this test is the only place it's called at the moment)? Could this become an epicsAtExit() routine, or would that just bring back the shutdown delays that caused these changes in the first place? Cleaning up things which only get initialized in an epicsThreadOnce() routine makes them impossible to use afterwards, so you couldn't call ipAddrToAsciiEngine::cleanup() in dbUnitTest.c; I'm not really sure where it could be used.

I'm not really expecting to be able to run the self-tests more than once without rebooting, but it is nice if they don't crash since many of the tests /can/ be run more than once.

There doesn't appear to be any documentation for this subsystem that could be updated, but the previous fix was mentioned in the 3.14.12.6 Release Notes so it would be good to have this fix covered there too.

[mdavidsaver (mdavidsaver)]

> Is the ipAddrToAsciiEngine::cleanup() routine necessary ...

This exists to help valgrind to tell me if I'm doing something stupid. It can be skipped with no ill effects. Maybe #ifdef to omit on rtems/vxworks?

> Release Notes

ok

[mdavidsaver (mdavidsaver)]

@anj done

[Andrew Johnson (anj)]

Thanks. This branch is intended for merging into 3.14, which doesn't have valgrind/valgrind.h (nor does the 3.15 branch), so my build died:

../ipAddrToAsciiTest.cpp:10:31: fatal error: valgrind/valgrind.h: No such file or directory
 #include <valgrind/valgrind.h>
                               ^
compilation terminated.

However the failure didn't happen until it got to my windows-x64-mingw cross-compile; I got warnings from the VxWorks and RTEMS builds about the missing file, but they still succeeded because it isn't actually necessary to pull in that include file at all. The cleanup() routine is only useful for testing under valgrind, which only runs on Linux, so I just changed the conditional around the call to #ifdef __linux__ and now everything works.

Merging.

review: Approve

=== modified file 'src/libCom/misc/ipAddrToAsciiAsynchronous.cpp'
--- src/libCom/misc/ipAddrToAsciiAsynchronous.cpp	2016-09-02 20:12:50 +0000
+++ src/libCom/misc/ipAddrToAsciiAsynchronous.cpp	2017-02-27 16:54:43 +0000
@@ -18,6 +18,9 @@
 #include <stdexcept>
 #include <stdio.h>
 
+//#define EPICS_FREELIST_DEBUG
+#define EPICS_PRIVATE_API
+
 #define epicsExportSharedSymbols
 #include "ipAddrToAsciiAsynchronous.h"
 #include "epicsThread.h"
@@ -45,7 +48,6 @@
         < ipAddrToAsciiTransactionPrivate, 0x80 > & );
     epicsPlacementDeleteOperator (( void *, tsFreeList 
         < ipAddrToAsciiTransactionPrivate, 0x80 > & ))
-private:
     osiSockAddr addr;
     ipAddrToAsciiEnginePrivate & engine;
     ipAddrToAsciiCallBack * pCB;
@@ -54,7 +56,7 @@
     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 @@
 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 @@
 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<epicsMutex> 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 @@
         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 <const void *> (this), this->labor.count () );
+        static_cast <const void *> (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 @@
     }
     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 @@
 
 ipAddrToAsciiTransaction & ipAddrToAsciiEnginePrivate::createTransaction ()
 {
-    return * new ( this->transactionFreeList ) ipAddrToAsciiTransactionPrivate ( *this );
+    epicsGuard <epicsMutex> 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 @@
             // 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 @@
             }
 
             this->callbackInProgress = false;
+            pActive = 0;
 
             if ( this->pCurrent ) {
                 this->pCurrent->pending = false;
@@ -300,44 +375,53 @@
 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 ();
-                }
-                break;
-            }
-        }
-        else {
-            if ( this->engine.pCurrent == this ) {
-                // cancel from callback, or while lookup in progress
-                this->engine.pCurrent = 0;
+    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;
+                }
             }
             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 @@
     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 @@
     }
 
     if ( success ) {
-        this->engine.laborEvent.signal ();
+        pGlobal->laborEvent.signal ();
     }
     else {
         char autoNameTmp[256];
@@ -379,7 +469,7 @@
 
 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 );

=== modified file 'src/libCom/misc/ipAddrToAsciiAsynchronous.h'
--- src/libCom/misc/ipAddrToAsciiAsynchronous.h	2016-05-22 03:43:09 +0000
+++ src/libCom/misc/ipAddrToAsciiAsynchronous.h	2017-02-27 16:54:43 +0000
@@ -44,6 +44,10 @@
     static ipAddrToAsciiEngine & allocate ();
 protected:
     virtual ~ipAddrToAsciiEngine () = 0;
+public:
+#ifdef EPICS_PRIVATE_API
+    static void cleanup();
+#endif
 };
 
 #endif // ifdef ipAddrToAsciiAsynchronous_h

=== modified file 'src/libCom/test/Makefile'
--- src/libCom/test/Makefile	2016-07-06 16:36:09 +0000
+++ src/libCom/test/Makefile	2017-02-27 16:54:43 +0000
@@ -151,6 +151,9 @@
 testHarness_SRCS += epicsMessageQueueTest.cpp
 TESTS += epicsMessageQueueTest
 
+TESTPROD_HOST += ipAddrToAsciiTest
+ipAddrToAsciiTest_SRCS += ipAddrToAsciiTest.cpp
+TESTS += ipAddrToAsciiTest
 
 # The testHarness runs all the test programs in a known working order.
 testHarness_SRCS += epicsRunLibComTests.c

=== added file 'src/libCom/test/ipAddrToAsciiTest.cpp'
--- src/libCom/test/ipAddrToAsciiTest.cpp	1970-01-01 00:00:00 +0000
+++ src/libCom/test/ipAddrToAsciiTest.cpp	2017-02-27 16:54:43 +0000
@@ -0,0 +1,161 @@
+/*************************************************************************\
+* 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 <stdio.h>
+#include <stdlib.h>
+
+#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<epicsMutex> Guard;
+typedef epicsGuardRelease<epicsMutex> 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);
+    ipAddrToAsciiEngine::cleanup();
+    return testDone();
+}