ring buffer overflows -> cDevice::Detach() blockiert...

Somebody should post this issue on the VDR mailing list so that Klaus gets aware about that...

Cheers Louis

Erledigt .

Das Einzige, was mir dazu einfällt, ist diese Änderung, die in Version 2.1.7 eingeflossen ist:

--- device.c	2014/03/15 13:23:28	3.15
+++ device.c	2015/01/07 12:53:55	3.16
@@ -1699,10 +1699,11 @@
          receiversLeft = true;
       }
   if (camSlot) {
-     if (Receiver->priority > MINPRIORITY) // priority check to avoid an infinite loop with the CAM slot's caPidReceiver
+     if (Receiver->priority > MINPRIORITY) { // priority check to avoid an infinite loop with the CAM slot's caPidReceiver
         camSlot->StartDecrypting();
-     if (!camSlot->IsDecrypting())
-        camSlot->Assign(NULL);
+        if (!camSlot->IsDecrypting())
+           camSlot->Assign(NULL);
+        }
      }
   if (!receiversLeft)
      Cancel(-1);

Aber nachdem Xcoder ja VDR 2.2.0 benutzt ist dieser Code ja offensichtlich in seiner Version enthalten.

Mehr kann ich dazu im Moment leider auch nicht beitragen.

Klaus

jrie,

the {] is to put the mutex in a block, at the end of the block the mutexlock will be freeed.

{ //RB prevent deadlock
cMutexLock MutexLock(&mutexReceiver); //RB
Lock();
} //RB

Ich sehe es ähnlich wie rvdb es fehlt ein vorzeitiges Unlock und zwar indirekt über {}. Wie es aussieht, startet der Aufruf "camSlot->StartDecrypting()" in Zeile 1706 einen neuen Threadaufruf und dieser führt indirekt zum Deadlock. Da dieser aber nicht über Mutex geschützt werden muss, sollte der Mutex früher freigegeben werden, also

Zeile #1690

{ //Neu
cMutexLock MutexLock(&mutexReceiver);
for (int i = 0; i < MAXRECEIVERS; i++) {
if (receiver[i] == Receiver) {
Lock();
receiver[i] = NULL;
Receiver->device = NULL;
Unlock();
Receiver->Activate(false);
for (int n = 0; n < Receiver->numPids; n++)
DelPid(Receiver->pids[n]);
}
else if (receiver[i])
receiversLeft = true;
}
} //Neu

Sagen wir einmal so, es schadet auch nicht, dies so zu tun. Ich sehen nur eine Abhängigkeit über den Aufruf "StartDecrypting", dass beide Threads dann über Aufrufe in "ci.c" landen, Thread 1 mit Aufruf "SendCaPmt" und Thread 159 "Receive" und beide führen dann "Detach" durch.

Bei einem Deadlock müsste ein Thread ein Lock gemacht haben, aber noch kein Unlock. Vielleicht fehlt in der Aufzeichnung hier noch ein dritter Thread, welcher dies gemacht hat und wir sehen nur die Auswirkung.

Von der Analyse der backtraces in diesem Thread her sehe ich es so:

Der main-thread hängt nach dem Kanal-Wechsel den Plugin-Receiver ab (1. Detach in main), und dadurch wird der camSlot abgehängt (2. Detach in main).
Der Action()-thread von device.c verteilt die Daten an die Receiver über Receive(…), die werden wieder abgehängt (Detach im Action-thread).

Der main-thread bleibt im Lock() vom 2. Detach stehen und hat vorher mutexReceiver gelockt.
Der Action()-thread bleibt im cMutexLock::Lock (aufgerufen von cMutexLock MutexLock(&mutexReceiver) ) von seinem Detach stehen, wartet also wegen des mutexReceiver.
Der Action()-thread hat aber vorher Lock() aufgerufen, und deshalb wartet der main-thread.
=> deadlock!

main:
Detach
mutexReceiver #1690
Lock() #1693

Action:
Lock() #1605
Receive
Detach
mutexReceiver #1690

1690 <-> 1690 und 1605 <-> 1693 beissen sich.

Zitat von xyzzy

{ //RB prevent deadlock
cMutexLock MutexLock(&mutexReceiver); //RB
Lock();
} //RB

Also with this I get sometimes a black screen after channel switch (no streamdev involved).

jrie, genau das meine ich, vielleicht nicht so schön beschrieben. Wie ich es verstehe sollte mit "vcMutexLock MutexLock(&mutexReceiver)" nur "receiver[i]" geschützt werden. Daher bin ich der selben Meinung wie rvdb, dass um vcMutexLock MutexLock(&mutexReceiver)"eine {} gehört und zwar nur, wo auf "receiver[i] zugegriffen wird.

Es wäre schön, wenn man nun ein Backtrace mit der Änderung bekommen könnte. Warum aber nun vermehrt ein "Black Screen" erscheint, sieht mir nach einem Nebeneffekt aus.

Manchmal kommt mit der Änderung kein Bild nach dem Umschalten.
Interessanterweise kommt das Bild in dem Fall sofort, wenn ich das femon Plugin starte.

Mir ist noch nicht eingefallen, wie man das sauber löst. (Also Sicherzustellen, dass in receiver[ i]->Receive(...) der receiver[ i] noch da ist, ohne dass es zum deadlock kommt im Detach(...) ).

Habe leider erst mal keine Zeit mehr dafür.

Xcoder: Bringt denn die Änderung von xyzzy was?
Ich habe seine Änderung, so wie ich sie verstehe, mal angehängt:

rvdb: Just to be sure, is the above patch actually what you meant?

Ich verstehe den Vorschlag etwas anders hier die gesamte Funktion als Lösung

void cDevice::Detach(cReceiver *Receiver)
{
  if (!Receiver || Receiver->device != this)
 	return;
  bool receiversLeft = false;
  {//neu Beginn des Gültigkeitsbereich von MutexLock
	cMutexLock MutexLock(&mutexReceiver);
	for (int i = 0; i < MAXRECEIVERS; i++) {
		if (receiver[i] == Receiver) {
			Lock();
			receiver[i] = NULL;
			Receiver->device = NULL;
			Unlock();
			Receiver->Activate(false);
			for (int n = 0; n < Receiver->numPids; n++)
				DelPid(Receiver->pids[n]);
			}
		else if (receiver[i])
			receiversLeft = true;
		}
   }//hier endet der Gültigkeitsbereich von MutexLock
  //ab hier ist nun kein Lock mehr aktiv    
  if (camSlot) {
 	if (Receiver->priority > MINPRIORITY) { // priority check to avoid an infinite loop with the CAM slot's caPidReceiver
    	camSlot->StartDecrypting();
    	if (!camSlot->IsDecrypting() && !camSlot->IsActivating())
       	camSlot->Assign(NULL);
    	}
 	}
  if (!receiversLeft)
 	Cancel(-1);
}

jrie, Yes thats what I have.
(only Lock if the receiver mutex is free, so a side effect is that we could get
an extra delay here if the other thread is holding the mutex.

Do yo have the black screen on al channels ? or only the encrypted ones ?
I sometimes have a black screen also but only for about 10 seconds !
(missing an ecm ????)

greetings

Guido17

Auch mit dieser Änderung bleibt der vdr hängen, so man zur Beschleunigung eine Aufnahme startet:

May 22 17:08:50 sara-hd vdr: [4180] ERROR: 1 ring buffer overflow (1128 bytes dropped)
May 22 17:08:51 sara-hd vdr: [4180] SATIP: Detected 1 RTP packet errors [device 0]
May 22 17:08:56 sara-hd vdr: [4180] ERROR: 554 ring buffer overflows (729064 bytes dropped)
May 22 17:09:02 sara-hd vdr: [4180] ERROR: 593 ring buffer overflows (780388 bytes dropped)
May 22 17:09:08 sara-hd vdr: [4180] ERROR: 593 ring buffer overflows (780388 bytes dropped)
May 22 17:09:14 sara-hd vdr: [4180] ERROR: 593 ring buffer overflows (780388 bytes dropped)
May 22 17:09:19 sara-hd vdr: [4246] ERROR: video data stream broken
May 22 17:09:19 sara-hd vdr: [4246] initiating emergency exit

Blöd...

Grüße,
42

Zitat von xyzzy

Do yo have the black screen on al channels ? or only the encrypted ones ?
I sometimes have a black screen also but only for about 10 seconds !
(missing an ecm ????)

I had the black screens on free channels. Just after a few switches. And I waited for maybe 1 or 2 minutes.
I wonder, why exactly calling femon helps. Must have something to do with that femon attaches a receiver, too.
In the very moment, when I start femon, the black screen turns into regular picture.

When I start VDR with the osdteletext-plugin, I see no more black screens (just a short test). I guess because osdteletext does automatically, what femon does: attaching a receiver.

rvdb, Xcoder: If you start VDR with as few plugins as possible, do you get black screens too?
If yes, that would mean, the patch is not ok for plain VDR.

Even with the patch a deadlock could still happen (although less often): Suppose in Action() you have passed the Lock(), but before you reach the Detach(...) after Receive() there happens a Detach(...) in main().
Am I right, or do I miss something?

In my opinion there is no solution possible, because of the order of the calls.
The only thing I see, is to turn the
Lock()
…
Unlock()
in Action() into
{
cMutexLock MutexLock(&mutexReceiver
….
}
see attached patch.

Xcoder: Testest du den mal?

Aber auch damit gibt es bei mir Black Screens.
Dafür brauchen wir noch was.