[2.1.2] Segfault, wenn das Videoverzeichnis aktualisiert wird, während das Aufnahmenmenü offen ist

Moin!

Es passiert nicht immer, lässt sich aber mit mehreren Versuchen immer mal wieder provozieren.
Aufnahmenmenü öffnen, Auswahl auf einer Aufnahme platzieren, "svdrpsend updr" ausführen.
Evtl. muss "LastReplayed" in der setup.conf auch noch den passenden Namen haben, das hab ich noch nicht verifiziert.

(gdb) bt
#0  __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:32
#1  0x00007fc4d2184d76 in __GI___strdup (s=0x0) at strdup.c:42
#2  0x00000000004d610b in cRecording::FileName (this=0x7fc49407dba0) at recording.c:1027
#3  0x00000000004b8515 in cMenuRecordings::Set (this=0x233f260, Refresh=true) at menu.c:2610
#4  0x00000000004ba340 in cMenuRecordings::ProcessKey (this=0x233f260, Key=kNone) at menu.c:2827
#5  0x00000000004c4818 in cOsdMenu::ProcessKey (this=0x233f1a0, Key=kNone) at osdbase.c:519
#6  0x00000000004ba1e6 in cMenuRecordings::ProcessKey (this=0x233f1a0, Key=kNone) at menu.c:2812
#7  0x00000000004c4818 in cOsdMenu::ProcessKey (this=0x2335280, Key=kNone) at osdbase.c:519
#8  0x00000000004ba1e6 in cMenuRecordings::ProcessKey (this=0x2335280, Key=kNone) at menu.c:2812
#9  0x00000000004c4818 in cOsdMenu::ProcessKey (this=0x22a35a0, Key=kNone) at osdbase.c:519
#10 0x00000000004ba1e6 in cMenuRecordings::ProcessKey (this=0x22a35a0, Key=kNone) at menu.c:2812
#11 0x00000000004c4818 in cOsdMenu::ProcessKey (this=0x22c61f0, Key=kNone) at osdbase.c:519
#12 0x00000000004b8ea2 in cMenuMain::ProcessKey (this=0x22c61f0, Key=kNone) at menu.c:3861
#13 0x000000000046d0f6 in main (argc=<optimized out>, argv=<optimized out>) at vdr.c:1217
(gdb) f 2
#2  0x00000000004d610b in cRecording::FileName (this=0x7fc49407dba0) at recording.c:1027
1027         char *Name = LimitNameLengths(strdup(name), DirectoryPathMax - strlen(cVideoDirectory::Name()) - 1 - 42, DirectoryNameMax); // 42 = length of an actual recording directory name (generated with DATAFORMATTS) plus some reserve
(gdb) p *this
$7 = {<cListObject> = {_vptr.cListObject = 0x0, prev = 0x0, next = 0x0}, resume = 0, titleBuffer = 0x0, sortBufferName = 0x0, sortBufferTime = 0x0,
  fileName = 0x0, name = 0x0, fileSizeMB = 0, numFrames = 0, channel = 0, instanceId = 0, isPesRecording = false, isOnVideoDirectoryFileSystem = 0,
  framesPerSecond = 0, info = 0x0, start = 0, priority = 0, lifetime = 0, deleted = 0}

2603: void cMenuRecordings::Set(bool Refresh)
2604: {
2605:   const char *CurrentRecording = *fileName ? *fileName : cReplayControl::LastReplayed();
2606:   cMenuRecordingItem *LastItem = NULL;
2607:   cThreadLock RecordingsLock(&Recordings);
2608:   if (Refresh) {
2609:      if (cMenuRecordingItem *ri = (cMenuRecordingItem *)Get(Current()))
2610:         CurrentRecording = ri->Recording()->FileName();
2611:      }

UPDR löscht cRecordings und scannt das Videoverzeichnis komplett neu ein.
Die cMenuRecordingItem merken sich aber einen Zeiger auf ein cRecording-Objekt. Das existiert dann natürlich nicht mehr.
Außerdem merkt sich cReplayControl auch keine Kopie des Dateinamen, sondern einen Zeiger auf cRecording::fileName. Wenn aber später cReplayControl::LastReplayed aufgerufen wird, kommt ein Zeiger ins Nirvana zurück.
Ich hatte nämlich nicht nur an dieser Stelle einen Absturz, sondern auch etwas später in Zeile 2641 beim strcmp mit "CurrentRecording", welches dann einen Wert von 0x85 hatte.

Sollte cMenuRecordingItem sich eine Kopie des cRecording-Objektes erstellen? Es ist ja immerhin ein "flüchtiges" vdr-Objekt, sprich, kann jederzeit zerstört werden, wenn es nicht gelockt ist.
Oder sollte der Video-Scanner nicht starten, wenn das Aufnahmenmenü offen ist? Das halte ich aber für die falsche Lösung.
Ein Workaround wäre (da cMenuRecording::Set ja in diesem Moment einen Lock auf cRecordings hält), dass erst geprüft wird, ob ri->Recording() noch Bestandteil von cRecordings ist. Und wenn nicht, wird es einfach nicht verwendet. Hilft aber auch nicht beim cReplayControl.
Ich hab also noch keine Lösung, wollte das Problem aber schon mal mitteilen.

Ich weiß nicht, ob vergleichbares für cTimer und cEvent und deren Menüs gilt, momentan arbeite ich nur mit dem Aufnahmenmenü wegen des Patches, um die erste Verzeichnisebene auszublenden. Ich dachte schon, der wäre Schuld, aber ich hab das mit einem vanilla-vdr nachvollziehen können.

Lars.

Alles klar. Ich bin mir auch nicht sicher, ob das nicht in vdr 2.0.4 auch schon auftritt. So viel hat sich an der Stelle ja nicht geändert, auch wenn da ein paar Dinge hinzugekommen sind, die hier aber nicht hineinspielen.

Lars.

Ich sehe gerade, dass cMenuRecording::RefreshRecording da schon was interessantes und evtl. passendes macht:

bool cMenuRecording::RefreshRecording(void)
{
  if (Recordings.StateChanged(recordingsState)) {
     if ((recording = Recordings.GetByName(originalFileName)) != NULL)
        Display();
     else {
        Skins.Message(mtWarning, tr("Recording vanished!"));
        return false;
        }
     }
  return true;
}

Lars.

Moin!

Ich hab mal einen Patch erstellt. Basis ist das Verhalten in cMenuRecording.
Ich habe eine neue Klasse "cRecordingItem" eingeführt, die das cRecording-Objekt kapselt und bei Bedarf überprüft, ob es noch vorhanden ist. Danach muss nur noch an ein paar Stellen auf "!= NULL" geprüft werden.

Ist nur ein Ansatz, eine Idee. Die neue Klasse könnten dann auch Plugins usw. nutzen, um sich cRecording-Objekte zu merken.
cMenuRecording könnte sie natürlich genauso verwenden.

Bisher konnte ich keinen weitern Absturz provozieren. In cReplayControl hab ich das aber noch nicht drin.

Lars.

Mit cReplayControl hab ich mich wahrscheinlich vertan, fileName ist vom Typ cString, das kopiert den Dateinamen, genauso wie auch fileName in cMenuRecordings.
Also hat das vermutlich doch was mit dem cRecording-Objekt zu tun gehabt haben.

Anbei ein etwas erweiterter Patch, bei dem auch cMenuRecording auf cRecordingItem umgestellt ist.

Lars.

Moin!

Ich hab an ein paar Stellen noch einen Lock auf Recordings eingebaut, wo ich es für sinnvoll erachte. Genau kann ich es nicht abschätzen, ob alle notwendig sind, oder nicht. Aber gestört haben sie in meinen Tests auch nicht.

Frohe Weihnachten!

Lars.

Ich kann es mir allerdings erst am 26. ansehen, bin dann erst wieder zu Hause.

Frohe Weihnacht!

Lars

Moin!

Hier funktioniert der Patch auch wunderbar. Ist ganz lustig, wenn man eine Aufnahme auf der Shell umbenennt und dann UPDR aufruft. Dann erscheint die Aufnahme unter dem neuen Namen im Menü und danach verschwindet die alte.
Das ist eine gute Idee, die Pointer zu "parken". Wenn sich ein Skin o.ä. nun einen Pointer auf ein cRecording-Objekt "eine Weile" merkt (während der Darstellung), dann sollte jetzt alles gut gehen.

Wäre es denkbar, für vdr 2.1.x evtl. cRecording um eine Methode "HasVanished" zu erweitern? Dann können cRecording-Nutzer zwischendurch mal prüfen, ob's das Objekt noch gibt und es ggf. schnell vergessen.
Ich hab jetzt aber noch keinen Anwendungsfall für sowas, ist nur eine Idee. Kann man später evtl. auch noch aufgreifen.

Lars.

Das funktioniert bei mir noch nicht, es erscheint erst die neue, dann verschwindet die alte Aufnahme.
Ich öffne das Aufnahmenmenü und navigiere in ein Verzeichnis mit drei Aufnahmen (drei Folgen einer Serie).
Dann benenne ich in einer Shell ein Verzeichnis um (mv ... ...).
Anschließend rufe ich "svdrpsend updr" auf. Dabei ist es egal, ob die umbenannte Aufnahme ausgewählt ist oder nicht.

Ich schau mal, ob ich herausfinde, warum das so ist.

Lars.

Ja, das ist mir auch gerade aufgefallen. Aber dann wird "ChangeState" doch nicht mehr aufgerufen, wenn ich eine Aufzeichnung einfach auf der Shell lösche, oder?
Müsste das "DoChangeState" nicht zusätzlich bei der Rekursion mitgegeben werden, damit dann am Ende das "ChangeState" aufgerufen werden kann?

EDIT: Oder man macht ein Feld draus.

Lars.

So in der Art:

class cRecordings : public cList<cRecording>, public cThread {
 private:
   static char *updateFileName;
   bool deleted;
   bool initial;
+  bool doChangeState;
   time_t lastUpdate;

void cRecordings::ScanVideoDir(const char *DirName, bool Foreground, int LinkLevel, int DirLevel)
 {
+  if (DirLevel == 0)
+     doChangeState = false;
...
+  if (doChangeState && DirLevel == 0)
      ChangeState();
 }

Lars.

Ja, da dachte ich auch dran. Das, was ich oben geschrieben hab, funktioniert zumindest schon mal (in meinem Fall).
Ich probiere das mit dem Rückgabewert auch noch mal.

Lars.