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

Proposed by mdavidsaver
Status: Merged
Merge reported by: Andrew Johnson
Merged at revision: not available
Proposed branch: lp:~epics-core/epics-base/fix-async-dns
Merge into: lp:~epics-core/epics-base/3.14
Diff against target: 603 lines (+339/-81)
4 files modified
src/libCom/misc/ipAddrToAsciiAsynchronous.cpp (+171/-81)
src/libCom/misc/ipAddrToAsciiAsynchronous.h (+4/-0)
src/libCom/test/Makefile (+3/-0)
src/libCom/test/ipAddrToAsciiTest.cpp (+161/-0)
To merge this branch: bzr merge lp:~epics-core/epics-base/fix-async-dns
Reviewer Review Type Date Requested Status
Andrew Johnson Approve
Ralph Lange Approve
Review via email: mp+318385@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Andrew Johnson (anj) wrote :

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

Revision history for this message
Ralph Lange (ralph-lange) wrote :

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
Revision history for this message
Andrew Johnson (anj) wrote :

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.

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

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

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

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...

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

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.

Revision history for this message
Andrew Johnson (anj) wrote :

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.

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

> 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

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

@anj done

Revision history for this message
Andrew Johnson (anj) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
18#include <stdexcept>18#include <stdexcept>
19#include <stdio.h>19#include <stdio.h>
2020
21//#define EPICS_FREELIST_DEBUG
22#define EPICS_PRIVATE_API
23
21#define epicsExportSharedSymbols24#define epicsExportSharedSymbols
22#include "ipAddrToAsciiAsynchronous.h"25#include "ipAddrToAsciiAsynchronous.h"
23#include "epicsThread.h"26#include "epicsThread.h"
@@ -45,7 +48,6 @@
45 < ipAddrToAsciiTransactionPrivate, 0x80 > & );48 < ipAddrToAsciiTransactionPrivate, 0x80 > & );
46 epicsPlacementDeleteOperator (( void *, tsFreeList 49 epicsPlacementDeleteOperator (( void *, tsFreeList
47 < ipAddrToAsciiTransactionPrivate, 0x80 > & ))50 < ipAddrToAsciiTransactionPrivate, 0x80 > & ))
48private:
49 osiSockAddr addr;51 osiSockAddr addr;
50 ipAddrToAsciiEnginePrivate & engine;52 ipAddrToAsciiEnginePrivate & engine;
51 ipAddrToAsciiCallBack * pCB;53 ipAddrToAsciiCallBack * pCB;
@@ -54,7 +56,7 @@
54 void release (); 56 void release ();
55 void * operator new ( size_t ); 57 void * operator new ( size_t );
56 void operator delete ( void * );58 void operator delete ( void * );
57 friend class ipAddrToAsciiEnginePrivate;59private:
58 ipAddrToAsciiTransactionPrivate & operator = ( const ipAddrToAsciiTransactionPrivate & );60 ipAddrToAsciiTransactionPrivate & operator = ( const ipAddrToAsciiTransactionPrivate & );
59 ipAddrToAsciiTransactionPrivate ( const ipAddrToAsciiTransactionPrivate & );61 ipAddrToAsciiTransactionPrivate ( const ipAddrToAsciiTransactionPrivate & );
60};62};
@@ -75,41 +77,54 @@
75static void ipAddrToAsciiEngineGlobalMutexConstruct ( void * );77static void ipAddrToAsciiEngineGlobalMutexConstruct ( void * );
76}78}
7779
78// - this class executes the synchronous DNS query80namespace {
79// - it creates one thread81struct ipAddrToAsciiGlobal : public epicsThreadRunable {
80class ipAddrToAsciiEnginePrivate : 82 ipAddrToAsciiGlobal();
81 public ipAddrToAsciiEngine, 83 virtual ~ipAddrToAsciiGlobal() {}
82 public epicsThreadRunable {84
83public:85 virtual void run ();
84 ipAddrToAsciiEnginePrivate ();86
85 virtual ~ipAddrToAsciiEnginePrivate ();
86 void show ( unsigned level ) const;
87private:
88 char nameTmp [1024];87 char nameTmp [1024];
89 tsFreeList 88 tsFreeList
90 < ipAddrToAsciiTransactionPrivate, 0x80 > 89 < ipAddrToAsciiTransactionPrivate, 0x80 >
91 transactionFreeList;90 transactionFreeList;
92 tsDLList < ipAddrToAsciiTransactionPrivate > labor;91 tsDLList < ipAddrToAsciiTransactionPrivate > labor;
93 mutable epicsMutex mutex;92 mutable epicsMutex mutex;
94 epicsEvent laborEvent;93 epicsEvent laborEvent;
95 epicsEvent destructorBlockEvent;94 epicsEvent destructorBlockEvent;
96 epicsThread thread;95 epicsThread thread;
96 // pCurrent may be changed by any thread (worker or other)
97 ipAddrToAsciiTransactionPrivate * pCurrent;97 ipAddrToAsciiTransactionPrivate * pCurrent;
98 // pActive may only be changed by the worker
99 ipAddrToAsciiTransactionPrivate * pActive;
98 unsigned cancelPendingCount;100 unsigned cancelPendingCount;
99 bool exitFlag;101 bool exitFlag;
100 bool callbackInProgress;102 bool callbackInProgress;
101 static ipAddrToAsciiEnginePrivate * pEngine;103};
104}
105
106// - this class executes the synchronous DNS query
107// - it creates one thread
108class ipAddrToAsciiEnginePrivate :
109 public ipAddrToAsciiEngine {
110public:
111 ipAddrToAsciiEnginePrivate() :refcount(1u), released(false) {}
112 virtual ~ipAddrToAsciiEnginePrivate () {}
113 void show ( unsigned level ) const;
114
115 unsigned refcount;
116 bool released;
117
118 static ipAddrToAsciiGlobal * pEngine;
102 ipAddrToAsciiTransaction & createTransaction ();119 ipAddrToAsciiTransaction & createTransaction ();
103 void release (); 120 void release ();
104 void run ();121
105 ipAddrToAsciiEnginePrivate ( const ipAddrToAsciiEngine & );122private:
123 ipAddrToAsciiEnginePrivate ( const ipAddrToAsciiEngine & );
106 ipAddrToAsciiEnginePrivate & operator = ( const ipAddrToAsciiEngine & );124 ipAddrToAsciiEnginePrivate & operator = ( const ipAddrToAsciiEngine & );
107 friend class ipAddrToAsciiEngine;
108 friend class ipAddrToAsciiTransactionPrivate;
109 friend void ipAddrToAsciiEngineGlobalMutexConstruct ( void * );
110};125};
111126
112ipAddrToAsciiEnginePrivate * ipAddrToAsciiEnginePrivate :: pEngine = 0;127ipAddrToAsciiGlobal * ipAddrToAsciiEnginePrivate :: pEngine = 0;
113static epicsThreadOnceId ipAddrToAsciiEngineGlobalMutexOnceFlag = EPICS_THREAD_ONCE_INIT;128static epicsThreadOnceId ipAddrToAsciiEngineGlobalMutexOnceFlag = EPICS_THREAD_ONCE_INIT;
114129
115// the users are not required to supply a show routine130// the users are not required to supply a show routine
@@ -124,12 +139,24 @@
124static void ipAddrToAsciiEngineGlobalMutexConstruct ( void * )139static void ipAddrToAsciiEngineGlobalMutexConstruct ( void * )
125{140{
126 try{141 try{
127 ipAddrToAsciiEnginePrivate::pEngine = new ipAddrToAsciiEnginePrivate ();142 ipAddrToAsciiEnginePrivate::pEngine = new ipAddrToAsciiGlobal ();
128 }catch(std::exception& e){143 }catch(std::exception& e){
129 errlogPrintf("ipAddrToAsciiEnginePrivate ctor fails with: %s\n", e.what());144 errlogPrintf("ipAddrToAsciiEnginePrivate ctor fails with: %s\n", e.what());
130 }145 }
131}146}
132147
148void ipAddrToAsciiEngine::cleanup()
149{
150 {
151 epicsGuard<epicsMutex> G(ipAddrToAsciiEnginePrivate::pEngine->mutex);
152 ipAddrToAsciiEnginePrivate::pEngine->exitFlag = true;
153 }
154 ipAddrToAsciiEnginePrivate::pEngine->laborEvent.signal();
155 ipAddrToAsciiEnginePrivate::pEngine->thread.exitWait();
156 delete ipAddrToAsciiEnginePrivate::pEngine;
157 ipAddrToAsciiEnginePrivate::pEngine = 0;
158}
159
133// for now its probably sufficent to allocate one 160// for now its probably sufficent to allocate one
134// DNS transaction thread for all codes sharing161// DNS transaction thread for all codes sharing
135// the same process that need DNS services but we 162// the same process that need DNS services but we
@@ -141,41 +168,78 @@
141 ipAddrToAsciiEngineGlobalMutexConstruct, 0 );168 ipAddrToAsciiEngineGlobalMutexConstruct, 0 );
142 if(!ipAddrToAsciiEnginePrivate::pEngine)169 if(!ipAddrToAsciiEnginePrivate::pEngine)
143 throw std::runtime_error("ipAddrToAsciiEngine::allocate fails");170 throw std::runtime_error("ipAddrToAsciiEngine::allocate fails");
144 return * ipAddrToAsciiEnginePrivate::pEngine;171 return * new ipAddrToAsciiEnginePrivate();
145}172}
146173
147ipAddrToAsciiEnginePrivate::ipAddrToAsciiEnginePrivate () :174ipAddrToAsciiGlobal::ipAddrToAsciiGlobal () :
148 thread ( *this, "ipToAsciiProxy",175 thread ( *this, "ipToAsciiProxy",
149 epicsThreadGetStackSize(epicsThreadStackBig),176 epicsThreadGetStackSize(epicsThreadStackBig),
150 epicsThreadPriorityLow ),177 epicsThreadPriorityLow ),
151 pCurrent ( 0 ), cancelPendingCount ( 0u ), exitFlag ( false ), 178 pCurrent ( 0 ), pActive ( 0 ), cancelPendingCount ( 0u ), exitFlag ( false ),
152 callbackInProgress ( false )179 callbackInProgress ( false )
153{180{
154 this->thread.start (); // start the thread181 this->thread.start (); // start the thread
155}182}
156183
157ipAddrToAsciiEnginePrivate::~ipAddrToAsciiEnginePrivate ()
158{
159 {
160 epicsGuard < epicsMutex > guard ( this->mutex );
161 this->exitFlag = true;
162 }
163 this->laborEvent.signal ();
164 this->thread.exitWait ();
165}
166184
167void ipAddrToAsciiEnginePrivate::release ()185void ipAddrToAsciiEnginePrivate::release ()
168{186{
187 bool last;
188 {
189 epicsGuard < epicsMutex > guard ( this->pEngine->mutex );
190 if(released)
191 throw std::logic_error("Engine release() called again!");
192
193 // released==true prevents new transactions
194 released = true;
195
196 {
197 // cancel any pending transactions
198 tsDLIter < ipAddrToAsciiTransactionPrivate > it(pEngine->labor.firstIter());
199 while(it.valid()) {
200 ipAddrToAsciiTransactionPrivate *trn = it.pointer();
201 ++it;
202
203 if(this==&trn->engine) {
204 trn->pending = false;
205 pEngine->labor.remove(*trn);
206 }
207 }
208
209 // cancel transaction in lookup or callback
210 if (pEngine->pCurrent && this==&pEngine->pCurrent->engine) {
211 pEngine->pCurrent->pending = false;
212 pEngine->pCurrent = 0;
213 }
214
215 // wait for completion of in-progress callback
216 pEngine->cancelPendingCount++;
217 while(pEngine->pActive && this==&pEngine->pActive->engine
218 && ! pEngine->thread.isCurrentThread()) {
219 epicsGuardRelease < epicsMutex > unguard ( guard );
220 pEngine->destructorBlockEvent.wait();
221 }
222 pEngine->cancelPendingCount--;
223 if(pEngine->cancelPendingCount)
224 pEngine->destructorBlockEvent.signal();
225 }
226
227 assert(refcount>0);
228 last = 0==--refcount;
229 }
230 if(last) {
231 delete this;
232 }
169}233}
170234
171void ipAddrToAsciiEnginePrivate::show ( unsigned level ) const235void ipAddrToAsciiEnginePrivate::show ( unsigned level ) const
172{236{
173 epicsGuard < epicsMutex > guard ( this->mutex );237 epicsGuard < epicsMutex > guard ( this->pEngine->mutex );
174 printf ( "ipAddrToAsciiEngine at %p with %u requests pending\n", 238 printf ( "ipAddrToAsciiEngine at %p with %u requests pending\n",
175 static_cast <const void *> (this), this->labor.count () );239 static_cast <const void *> (this), this->pEngine->labor.count () );
176 if ( level > 0u ) {240 if ( level > 0u ) {
177 tsDLIterConst < ipAddrToAsciiTransactionPrivate > 241 tsDLIter < ipAddrToAsciiTransactionPrivate >
178 pItem = this->labor.firstIter ();242 pItem = this->pEngine->labor.firstIter ();
179 while ( pItem.valid () ) {243 while ( pItem.valid () ) {
180 pItem->show ( level - 1u );244 pItem->show ( level - 1u );
181 pItem++;245 pItem++;
@@ -183,10 +247,10 @@
183 }247 }
184 if ( level > 1u ) {248 if ( level > 1u ) {
185 printf ( "mutex:\n" );249 printf ( "mutex:\n" );
186 this->mutex.show ( level - 2u );250 this->pEngine->mutex.show ( level - 2u );
187 printf ( "laborEvent:\n" );251 printf ( "laborEvent:\n" );
188 this->laborEvent.show ( level - 2u );252 this->pEngine->laborEvent.show ( level - 2u );
189 printf ( "exitFlag boolean = %u\n", this->exitFlag );253 printf ( "exitFlag boolean = %u\n", this->pEngine->exitFlag );
190 printf ( "exit event:\n" );254 printf ( "exit event:\n" );
191 }255 }
192}256}
@@ -226,10 +290,20 @@
226290
227ipAddrToAsciiTransaction & ipAddrToAsciiEnginePrivate::createTransaction ()291ipAddrToAsciiTransaction & ipAddrToAsciiEnginePrivate::createTransaction ()
228{292{
229 return * new ( this->transactionFreeList ) ipAddrToAsciiTransactionPrivate ( *this );293 epicsGuard <epicsMutex> G(this->pEngine->mutex);
294 if(this->released)
295 throw std::logic_error("createTransaction() on release()'d ipAddrToAsciiEngine");
296
297 assert(this->refcount>0);
298
299 ipAddrToAsciiTransactionPrivate *ret = new ( this->pEngine->transactionFreeList ) ipAddrToAsciiTransactionPrivate ( *this );
300
301 this->refcount++;
302
303 return * ret;
230}304}
231305
232void ipAddrToAsciiEnginePrivate::run ()306void ipAddrToAsciiGlobal::run ()
233{307{
234 epicsGuard < epicsMutex > guard ( this->mutex );308 epicsGuard < epicsMutex > guard ( this->mutex );
235 while ( ! this->exitFlag ) {309 while ( ! this->exitFlag ) {
@@ -267,7 +341,7 @@
267 // fix for lp:1580623341 // fix for lp:1580623
268 // a destructing cac sets pCurrent to NULL, so342 // a destructing cac sets pCurrent to NULL, so
269 // make local copy to avoid race when releasing the guard343 // make local copy to avoid race when releasing the guard
270 ipAddrToAsciiTransactionPrivate *pCur = this->pCurrent;344 ipAddrToAsciiTransactionPrivate *pCur = pActive = pCurrent;
271 this->callbackInProgress = true;345 this->callbackInProgress = true;
272346
273 {347 {
@@ -277,6 +351,7 @@
277 }351 }
278352
279 this->callbackInProgress = false;353 this->callbackInProgress = false;
354 pActive = 0;
280355
281 if ( this->pCurrent ) {356 if ( this->pCurrent ) {
282 this->pCurrent->pending = false;357 this->pCurrent->pending = false;
@@ -300,44 +375,53 @@
300void ipAddrToAsciiTransactionPrivate::release ()375void ipAddrToAsciiTransactionPrivate::release ()
301{376{
302 this->~ipAddrToAsciiTransactionPrivate ();377 this->~ipAddrToAsciiTransactionPrivate ();
303 this->engine.transactionFreeList.release ( this );378 this->engine.pEngine->transactionFreeList.release ( this );
304}379}
305380
306ipAddrToAsciiTransactionPrivate::~ipAddrToAsciiTransactionPrivate ()381ipAddrToAsciiTransactionPrivate::~ipAddrToAsciiTransactionPrivate ()
307{382{
308 epicsGuard < epicsMutex > guard ( this->engine.mutex );383 ipAddrToAsciiGlobal *pGlobal = this->engine.pEngine;
309 while ( this->pending ) {384 bool last;
310 if ( this->engine.pCurrent == this && 385 {
311 this->engine.callbackInProgress && 386 epicsGuard < epicsMutex > guard ( pGlobal->mutex );
312 ! this->engine.thread.isCurrentThread() ) {387 while ( this->pending ) {
313 // cancel from another thread while callback in progress388 if ( pGlobal->pCurrent == this &&
314 // waits for callback to complete389 pGlobal->callbackInProgress &&
315 assert ( this->engine.cancelPendingCount < UINT_MAX );390 ! pGlobal->thread.isCurrentThread() ) {
316 this->engine.cancelPendingCount++;391 // cancel from another thread while callback in progress
317 {392 // waits for callback to complete
318 epicsGuardRelease < epicsMutex > unguard ( guard );393 assert ( pGlobal->cancelPendingCount < UINT_MAX );
319 this->engine.destructorBlockEvent.wait ();394 pGlobal->cancelPendingCount++;
320 }395 {
321 assert ( this->engine.cancelPendingCount > 0u );396 epicsGuardRelease < epicsMutex > unguard ( guard );
322 this->engine.cancelPendingCount--;397 pGlobal->destructorBlockEvent.wait ();
323 if ( ! this->pending ) {398 }
324 if ( this->engine.cancelPendingCount ) {399 assert ( pGlobal->cancelPendingCount > 0u );
325 this->engine.destructorBlockEvent.signal ();400 pGlobal->cancelPendingCount--;
326 }401 if ( ! this->pending ) {
327 break;402 if ( pGlobal->cancelPendingCount ) {
328 }403 pGlobal->destructorBlockEvent.signal ();
329 }404 }
330 else {405 break;
331 if ( this->engine.pCurrent == this ) {406 }
332 // cancel from callback, or while lookup in progress
333 this->engine.pCurrent = 0;
334 }407 }
335 else {408 else {
336 // cancel before lookup starts409 if ( pGlobal->pCurrent == this ) {
337 this->engine.labor.remove ( *this );410 // cancel from callback, or while lookup in progress
411 pGlobal->pCurrent = 0;
412 }
413 else {
414 // cancel before lookup starts
415 pGlobal->labor.remove ( *this );
416 }
417 this->pending = false;
338 }418 }
339 this->pending = false;
340 }419 }
420 assert(this->engine.refcount>0);
421 last = 0==--this->engine.refcount;
422 }
423 if(last) {
424 delete &this->engine;
341 }425 }
342}426}
343427
@@ -345,15 +429,21 @@
345 const osiSockAddr & addrIn, ipAddrToAsciiCallBack & cbIn )429 const osiSockAddr & addrIn, ipAddrToAsciiCallBack & cbIn )
346{430{
347 bool success;431 bool success;
432 ipAddrToAsciiGlobal *pGlobal = this->engine.pEngine;
348433
349 {434 {
350 epicsGuard < epicsMutex > guard ( this->engine.mutex );435 epicsGuard < epicsMutex > guard ( pGlobal->mutex );
351 // put some reasonable limit on queue expansion436
352 if ( !this->pending && engine.labor.count () < 16u ) {437 if (this->engine.released) {
438 errlogPrintf("Warning: ipAddrToAscii on transaction with release()'d ipAddrToAsciiEngine");
439 success = false;
440
441 } else if ( !this->pending && pGlobal->labor.count () < 16u ) {
442 // put some reasonable limit on queue expansion
353 this->addr = addrIn;443 this->addr = addrIn;
354 this->pCB = & cbIn;444 this->pCB = & cbIn;
355 this->pending = true;445 this->pending = true;
356 this->engine.labor.add ( *this );446 pGlobal->labor.add ( *this );
357 success = true;447 success = true;
358 }448 }
359 else {449 else {
@@ -362,7 +452,7 @@
362 }452 }
363453
364 if ( success ) {454 if ( success ) {
365 this->engine.laborEvent.signal ();455 pGlobal->laborEvent.signal ();
366 }456 }
367 else {457 else {
368 char autoNameTmp[256];458 char autoNameTmp[256];
@@ -379,7 +469,7 @@
379469
380void ipAddrToAsciiTransactionPrivate::show ( unsigned level ) const470void ipAddrToAsciiTransactionPrivate::show ( unsigned level ) const
381{471{
382 epicsGuard < epicsMutex > guard ( this->engine.mutex );472 epicsGuard < epicsMutex > guard ( this->engine.pEngine->mutex );
383 char ipAddr [64];473 char ipAddr [64];
384 sockAddrToDottedIP ( &this->addr.sa, ipAddr, sizeof ( ipAddr ) );474 sockAddrToDottedIP ( &this->addr.sa, ipAddr, sizeof ( ipAddr ) );
385 printf ( "ipAddrToAsciiTransactionPrivate for address %s\n", ipAddr );475 printf ( "ipAddrToAsciiTransactionPrivate for address %s\n", ipAddr );
386476
=== 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 @@
44 static ipAddrToAsciiEngine & allocate ();44 static ipAddrToAsciiEngine & allocate ();
45protected:45protected:
46 virtual ~ipAddrToAsciiEngine () = 0;46 virtual ~ipAddrToAsciiEngine () = 0;
47public:
48#ifdef EPICS_PRIVATE_API
49 static void cleanup();
50#endif
47};51};
4852
49#endif // ifdef ipAddrToAsciiAsynchronous_h53#endif // ifdef ipAddrToAsciiAsynchronous_h
5054
=== 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 @@
151testHarness_SRCS += epicsMessageQueueTest.cpp151testHarness_SRCS += epicsMessageQueueTest.cpp
152TESTS += epicsMessageQueueTest152TESTS += epicsMessageQueueTest
153153
154TESTPROD_HOST += ipAddrToAsciiTest
155ipAddrToAsciiTest_SRCS += ipAddrToAsciiTest.cpp
156TESTS += ipAddrToAsciiTest
154157
155# The testHarness runs all the test programs in a known working order.158# The testHarness runs all the test programs in a known working order.
156testHarness_SRCS += epicsRunLibComTests.c159testHarness_SRCS += epicsRunLibComTests.c
157160
=== 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 @@
1/*************************************************************************\
2* Copyright (c) 2017 Michael Davidsaver
3* EPICS BASE is distributed subject to a Software License Agreement found
4* in file LICENSE that is included with this distribution.
5\*************************************************************************/
6
7#include <stdio.h>
8#include <stdlib.h>
9
10#define EPICS_PRIVATE_API
11
12#include "epicsMutex.h"
13#include "epicsGuard.h"
14#include "epicsThread.h"
15#include "epicsEvent.h"
16#include "ipAddrToAsciiAsynchronous.h"
17
18#include "epicsUnitTest.h"
19#include "testMain.h"
20
21namespace {
22
23typedef epicsGuard<epicsMutex> Guard;
24typedef epicsGuardRelease<epicsMutex> UnGuard;
25
26struct CB : public ipAddrToAsciiCallBack
27{
28 const char *name;
29 epicsMutex mutex;
30 epicsEvent starter, blocker, complete;
31 bool started, cont, done;
32 CB(const char *name) : name(name), started(false), cont(false), done(false) {}
33 virtual ~CB() {}
34 virtual void transactionComplete ( const char * pHostName )
35 {
36 Guard G(mutex);
37 started = true;
38 starter.signal();
39 testDiag("In transactionComplete(%s) for %s", pHostName, name);
40 while(!cont) {
41 UnGuard U(G);
42 if(!blocker.wait(2.0))
43 break;
44 }
45 done = true;
46 complete.signal();
47 }
48 void waitStart()
49 {
50 Guard G(mutex);
51 while(!started) {
52 UnGuard U(G);
53 if(!starter.wait(2.0))
54 break;
55 }
56 }
57 void poke()
58 {
59 testDiag("Poke");
60 Guard G(mutex);
61 cont = true;
62 blocker.signal();
63 }
64 void finish()
65 {
66 testDiag("Finish");
67 Guard G(mutex);
68 while(!done) {
69 UnGuard U(G);
70 if(!complete.wait(2.0))
71 break;
72 }
73 testDiag("Finished");
74 }
75};
76
77// ensure that lookup of 127.0.0.1 works
78void doLookup(ipAddrToAsciiEngine& engine)
79{
80 testDiag("In doLookup");
81
82 ipAddrToAsciiTransaction& trn(engine.createTransaction());
83 CB cb("cb");
84 osiSockAddr addr;
85 addr.ia.sin_family = AF_INET;
86 addr.ia.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
87 addr.ia.sin_port = htons(42);
88
89 testDiag("Start lookup");
90 trn.ipAddrToAscii(addr, cb);
91 cb.poke();
92 cb.finish();
93 testOk1(cb.cont);
94 testOk1(cb.done);
95
96 trn.release();
97}
98
99// Test cancel of pending transaction
100void doCancel()
101{
102 testDiag("In doCancel");
103
104 ipAddrToAsciiEngine& engine1(ipAddrToAsciiEngine::allocate());
105 ipAddrToAsciiEngine& engine2(ipAddrToAsciiEngine::allocate());
106
107 ipAddrToAsciiTransaction& trn1(engine1.createTransaction()),
108 & trn2(engine2.createTransaction());
109 testOk1(&trn1!=&trn2);
110 CB cb1("cb1"), cb2("cb2");
111
112 osiSockAddr addr;
113 addr.ia.sin_family = AF_INET;
114 addr.ia.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
115 addr.ia.sin_port = htons(42);
116
117 // ensure that the worker thread is blocked with a transaction from engine1
118 testDiag("Start lookup1");
119 trn1.ipAddrToAscii(addr, cb1);
120 testDiag("Wait start1");
121 cb1.waitStart();
122
123 testDiag("Start lookup2");
124 trn2.ipAddrToAscii(addr, cb2);
125
126 testDiag("release engine2, implicitly cancels lookup2");
127 engine2.release();
128
129 cb2.poke();
130 testDiag("Wait for lookup2 timeout");
131 cb2.finish();
132 testOk1(!cb2.done);
133
134 testDiag("Complete lookup1");
135 cb1.poke();
136 cb1.finish();
137 testOk1(cb1.done);
138
139 engine1.release();
140
141 trn1.release();
142 trn2.release();
143}
144
145} // namespace
146
147MAIN(ipAddrToAsciiTest)
148{
149 testPlan(5);
150 {
151 ipAddrToAsciiEngine& engine(ipAddrToAsciiEngine::allocate());
152 doLookup(engine);
153 engine.release();
154 }
155 doCancel();
156 // TODO: somehow test cancel of in-progress callback
157 // allow time for any un-canceled transcations to crash us...
158 epicsThreadSleep(1.0);
159 ipAddrToAsciiEngine::cleanup();
160 return testDone();
161}

Subscribers

People subscribed via source and target branches