Re: FW: Fixed: Segfaults in 3.14 branch since merging ca-over-tcp

[Ralph Lange <Ralph.Lange@bessy.de>]

 The bug that I fixed was introduced in the mentioned merge, which was 
after 3.14.11 was branched off.
Reason:
The line incrementing the iiuExistenceCount was added to cac.cpp after 
the revision that ca-over-tcp was originally based upon.
That particular code block (containing the increment) was refactored by 
ca-over-tcp into a separate function - that way it slipped through the 
merge, as that new function was not present in the pre-merge revision, 
and the (changed) code to compare the new function against was in a 
different location in the file.
The organized teardown is the only thing the iiuExistenceCount is used 
for. So I'd say the current version (using the fix that I sent earlier) 
is more obvious than the original code - as it sets the 
iiuExistenceCount in the method that uses it.
Probably a personal preference, though.

Ralph


On 16.08.2010 13:00, Kasemir, Kay wrote:
> Hi Ralph:
>
> We're using 3.14.11, and I wonder if this patch is also needed for us.
>
> In our sources, the
>
>                //
>                // shutdown all tcp circuits
>                //
> is around line 1230 of our src/ca/cac.cpp, not line 287 as in your email.
> It does look like the iiuExistenceCount is incremented and decremented in
> our code, but I don't try to understand if it's inc'ed/dec'ed correctly.
>
> So is this a patch that only applies to recent ca-over-tcp additions, not
> the plain 3.14.11?
>
> Thanks,
> Kay
>
>
> ------ Forwarded Message
> From: Ralph Lange<Ralph.Lange@bessy.de>
> Date: Mon, 16 Aug 2010 12:03:10 -0400
> To: Andrew Johnson<anj@aps.anl.gov>
> Cc: EPICS Core Talk<core-talk@aps.anl.gov>
> Subject: Fixed: Segfaults in 3.14 branch since merging ca-over-tcp
>
>    Hi Andrew,
>
> it looks as I have found the reason for the segfaults, which I think is
> a bug in CAC.
>
> In the cac::~cac() destructor, a private member
> "this->iiuExistenceCount" is used to determine if cac should wait for
> TCP threads to shutdown. While this counter gets correctly decreased in
> cac::destroyIIU(), it never gets increased or set (except initialized to
> 0 in the cpp constructor).
>
> So cac::~cac() was neve/r waiting for the TCP threads to shutdown, and
> happily kept tearing down the timer/mutex infrastructure. If the threads
> were not fast enough, they were hitting on destroyed semaphores when
> cancelling their watchdog timers - that was causing the exceptions and
> segfaults.
>
> This one-line fix will take care of the problem:
>
>
> === modified file 'src/ca/cac.cpp'
> --- src/ca/cac.cpp      2010-04-15 21:06:16 +0000
> +++ src/ca/cac.cpp      2010-08-16 15:24:13 +0000
> @@ -287,6 +287,7 @@
>                //
>                // shutdown all tcp circuits
>                //
> +            this->iiuExistenceCount = this->circuitList.count();
>                tsDLIter<  tcpiiu>  iter = this->circuitList.firstIter ();
>                while ( iter.valid() ) {
>                    // this causes a clean shutdown to occur
>
>
> Maybe it actually takes care of several shutdown related issues...
>
> Yay!
> Ralph
>
>
>
> ------ End of Forwarded Message