[Live-devel] StreamReplicator bug deactivating a replica?

[Bruno Abreu]

On 05/01/2012 02:32 AM, Ross Finlayson wrote:
> Try replacing those two "for" loops with the following:
>
> for (StreamReplica* r1 = fReplicasAwaitingCurrentFrame; r1 != NULL;) {
>   if (r1 == replicaBeingDeactivated) {
>     if (r1 == fReplicasAwaitingCurrentFrame) fReplicasAwaitingCurrentFrame =
>       r1->fNext;
>     r1 = r1->fNext;
>     replicaBeingDeactivated->fNext = NULL;
>     break;
>   } else {
>     r1 = r1->fNext;
>   }
> }
> for (StreamReplica* r2 = fReplicasAwaitingNextFrame; r2 != NULL;) {
>   if (r2 == replicaBeingDeactivated) {
>     if (r2 == fReplicasAwaitingNextFrame) fReplicasAwaitingNextFrame =
>       r2->fNext;
>     r2 = r2->fNext;
>    replicaBeingDeactivated->fNext = NULL;
>    break;
>   } else {
>     r2 = r2->fNext;
>   }
> }
>
> If you find any problem with this, then please let us know ASAP.
> Otherwise I'll release a new version of the code with this change.


Hello Ross.

After some days of testing I'm afraid I'm now pretty sure this solution 
(released on 2012.05.03) is still buggy.

Sorry I didn't report it earlier, but I also didn't see it sooner: can 
you see how, if the replica being removed is not the first on the list, 
the one preceding it will be left pointing at the removed one? It will!

The r1 and r2 pointers don't update the lists. That is done only if the 
replica being removed is the first element (r1 == 
fReplicasAwaitingCurrentFrame) or (r2 == fReplicasAwaitingNextFrame).

Using this solution we either get a segmentation fault or the following 
message on standard error:
"StreamReplicator::deliverReceivedFrame() Internal Error 2(2,2)!"

We've been successfully using the solution I proposed on my first 
message on this subject and which, I believe, is the most elegant (using 
a pointer to pointer to iterate over the lists), but the following has 
also worked flawlessly, although it is longer:

if (fReplicasAwaitingCurrentFrame != NULL) {
   if (replicaBeingDeactivated == fReplicasAwaitingCurrentFrame) {
     fReplicasAwaitingCurrentFrame = replicaBeingDeactivated->fNext;
     replicaBeingDeactivated->fNext = NULL;
   }
   else {
     for (StreamReplica* r1 = fReplicasAwaitingCurrentFrame; r1->fNext 
!= NULL; r1 = r1->fNext) {
       if (r1->fNext == replicaBeingDeactivated) {
         r1->fNext = replicaBeingDeactivated->fNext;
         replicaBeingDeactivated->fNext = NULL;
         break;
       }
     }
   }
}
if (fReplicasAwaitingNextFrame != NULL) {
   if (replicaBeingDeactivated == fReplicasAwaitingNextFrame) {
     fReplicasAwaitingNextFrame = replicaBeingDeactivated->fNext;
     replicaBeingDeactivated->fNext = NULL;
   }
   else {
     for (StreamReplica* r2 = fReplicasAwaitingNextFrame; r2->fNext != 
NULL; r2 = r2->fNext) {
       if (r2->fNext == replicaBeingDeactivated) {
         r2->fNext = replicaBeingDeactivated->fNext;
         replicaBeingDeactivated->fNext = NULL;
         break;
       }
     }
   }
}


Once again, sorry I didn't see this in time for the current release. 
Hope the next one will fix this issue.

Thank you,
Bruno Abreu

-- 
Living Data - Sistemas de Informação e Apoio à Decisão, Lda.

LxFactory - Rua Rodrigues de Faria, 103,
edifício I - 4º piso                  Phone:  +351 213622163
1300-501 LISBOA                       Fax:    +351 213622165
Portugal                              URL: www.livingdata.pt