Absturz wegen cEvent::title == NULL - Warum???

kls · December 2, 2024 at 2:37 PM

Quote from shofmann

Ich habe exemplarisch mal ein wenig in Live herumgestöbert und frage mich, ob zum Beispiel hier:
...
… nicht auch noch ein LOCK_SCHEDULES_READ gebräucht würde.

Die Funktion bekommt ja 'schedules' übergeben, welches nur über einen Lock erhalten werden konnte.

Also nein.

Quote from shofmann

Heißt das, dass man keine Locks setzen muss, solange man den zugeordneten Pointer (Timers, Channels, Schedules, Recordings) im Code nicht benötigt?

So ist es.

Quote from shofmann

Welche indirekte Abhängigkeiten gibt es diesbezüglich? Wie etwa die folgenden:

Du musst nur die Listen locken, mit denen du arbeiten möchtest. Wenn du mit mehreren Listen gleichzeitig arbeiten möchtest, musst du sie alle locken, und zwar in genau der genannten Reihenfolge, da es sonst zu Verklemmungen kommen könnte, wenn mehrere Threads auf mehrere Listen warten, sie aber in unterschiedlicher Reihenfolge locken.

kls · December 2, 2024 at 2:41 PM

Quote from FireFly

Wie sieht es mit dem realloc() in den beiden cString::Append() in tools.c aus?

Da sehe ich keine Probleme, da in den Daten-Members von cTimer, cChannel, cEvent und cRecording kein cString vorkommt.

kls · December 2, 2024 at 2:48 PM

Quote from shofmann

Auch wenn die meisten davon vermutlich eher unkritisch sind, sollte man sie bei dieser Gelegenheit wohl besser doch auf potentielle Race conditions bzw. Use-after-free-Szenarien überprüfen.

Werde ich selber aus Zeitgründen nicht machen, bin aber natürlich für jeden Bug-Report dankbar.

MarkusE · December 2, 2024 at 2:51 PM

Ich denke, hier wäre zusätzliche Dokumentation hilfreich.

Ist generell ein lesender Zugriff auf ein cEvent nur dann erlaubt, wenn ein read lock auf schedules gehalten wird?

Oder: Darf ich auf bestimmte Methoden eines cEvent zugreifen, ohne einen read lock auf schedules? Z.B. auf EventID()?

Und brauche den read lock auf schedules nur beim Zugriff auf "kritische" Methoden, z.B. Title(), ShortText(), ...?

~ Markus

kls · December 2, 2024 at 2:56 PM

MarkusE Sobald ein Pointer auf ein Objekt in einer dieser Listen dereferenziert wird, muss ein entsprechender Lock bestehen.

Siehe auch tools.h:

Code

class cListBase {
...
  bool Lock(cStateKey &StateKey, bool Write = false, int TimeoutMs = 0) const;
       ///< Tries to get a lock on this list and returns true if successful.
       ///< By default a read lock is requested. Set Write to true to obtain
       ///< a write lock. If TimeoutMs is not zero, it waits for the given
       ///< number of milliseconds before giving up.
       ///< If you need to lock more than one list at the same time, make sure
       ///< you set TimeoutMs to a suitable value in all of the calls to
       ///< Lock(), and be prepared to handle situations where you do not get all
       ///< of the requested locks. In such cases you should release all the locks
       ///< you have obtained so far and try again. StateKey.TimedOut() tells you
       ///< whether the lock attempt failed due to a timeout or because the state
       ///< of the lock hasn't changed since the previous locking attempt.
       ///< To implicitly avoid deadlocks when locking more than one of the global
       ///< lists of VDR at the same time, make sure you always lock Timers, Channels,
       ///< Recordings and Schedules in this sequence.
       ///< You may keep pointers to objects in this list, even after releasing
       ///< the lock. However, you may only access such objects if you are
       ///< holding a proper lock again. If an object has been deleted from the list
       ///< while you did not hold a lock (for instance by an other thread), the
       ///< object will still be there, but no longer within this list (it is then
       ///< stored in the ListGarbageCollector). That way even if you access the object
       ///< after it has been deleted, you won't cause a segfault. You can call the
       ///< Contains() function to check whether an object you are holding a pointer
       ///< to is still in the list. Note that the garbage collector is purged when
       ///< the usual housekeeping is done.

Display More

kls · December 2, 2024 at 3:20 PM

Es gibt noch eine Stelle, an der ich mich nicht an meine eigene Vorgabe gehalten habe

Diff

--- skinlcars.c 2024/09/21 10:53:07     5.7
+++ skinlcars.c 2024/12/02 14:15:55
@@ -1259,6 +1259,7 @@
      int NumDevices = 0;
      int y = ys04;
      // Timers and recording devices:
+     LOCK_SCHEDULES_READ;
      while (1) {
            int NumTimers = 0;
            const cDevice *Device = NULL;

SHofmann · December 2, 2024 at 5:10 PM

Quote from kls

Sobald ein Pointer auf ein Objekt in einer dieser Listen dereferenziert wird, muss ein entsprechender Lock bestehen.

Danke dir. Ich hatte so eine Antwort erwartet bzw. befürchtet. Da wird wohl in machen Plugins noch einiges zu berichtigten sein…

kfb77 · December 2, 2024 at 6:05 PM

Quote from shofmann

Heißt das, dass man keine Locks setzen muss, solange man den zugeordneten Pointer (Timers, Channels, Schedules, Recordings) im Code nicht benötigt?

Quote from kls

So ist es.

und

Quote from kls

Sobald ein Pointer auf ein Objekt in einer dieser Listen dereferenziert wird, muss ein entsprechender Lock bestehen.

Ist für mich gerade irgendwie ein Wiederspruch.

Beispiel:

Code

bool cEpgHandlerMarkad::HandleEvent(cEvent *Event) {
... irgendwelcher Code
    if (Event->RunningStatus() <= 0) return false;

Brauche ich da einen LOCK_SCHEDULES_READ oder kann ich davon ausgehen, dass das Objekt für die Dauer des Aufrufes unverändert bleibt ?

Ich nutze ja nicht die Liste, sondern nur ein einzelnes Objekt davon.

kls · December 2, 2024 at 6:09 PM

Wenn du ein Objekt übergeben bekommst, ist dessen Liste gelockt.

kfb77 · December 2, 2024 at 6:12 PM

Das macht Sinn. Danke.

SHofmann · December 3, 2024 at 12:07 PM

Quote from kfb77

Ich nutze ja nicht die Liste, sondern nur ein einzelnes Objekt davon.

… das sich zwischen den Zeilen 1 und 3 ja inzwischen schon wieder geändert haben oder gar gelöscht worden sein könnte. Insofern wäre mir der Lock einsichtiger, aber irgendwie bin ich jetzt noch mehr verwirrt.

Für mich stellt sich die Situation bislang so dar: Wann auch immer man auf eine der Listen zugreift (sprich: einen der besagten Pointer benötigt) oder (später) eines seiner Objekte referenziert, braucht man Locks auf die ursprüngliche Liste sowie die Listen aller Objekte, die davon direkt oder indirekt verlinkt sind.

Liege ich jetzt damit richtig, oder ist das noch immer nicht die ganze Wahrheit?

kfb77 · December 3, 2024 at 12:11 PM

Quote from shofmann

… das sich zwischen den Zeilen 1 und 3 ja inzwischen schon wieder geändert haben oder gar gelöscht worden sein könnte.

Nein, kann es nicht, da:

Quote from kls

Wenn du ein Objekt übergeben bekommst, ist dessen Liste gelockt.

FireFly · December 3, 2024 at 12:28 PM

genauer gesagt: Wenn du ein Objekt vom VDR übergeben bekommst, ist dessen Liste gelockt.

Wenn man selbst z.B. einen Event aus Schedules ändern will muss man Schedules locken solange man mit dem Event (-Pointer) arbeitet.

kls · December 3, 2024 at 3:57 PM

Quote from FireFly

genauer gesagt: Wenn du ein Objekt vom VDR übergeben bekommst, ist dessen Liste gelockt.

Ja, so war das natürlich gemeint.

kls · March 4, 2025 at 5:04 PM

Ich habe in den letzten Tagen noch einige Untersuchungen mit testausgaben gemacht und dabei ist manchmal der Fall aufgetreten, dass ein cEvent entsteht, bei dem title==NULL ist, der aber anscheinend nicht durch FixEpgBugs() geht. Warum das so ist, konnte ich nicht herausfinden. An fehlendem Locking scheint es wohl nicht (nehr) zu liegen. Daher nun als "ultima ratio":

Diff

--- epg.h       2025/03/02 11:03:35     5.9 
+++ epg.h       2025/03/04 15:54:07 
@@ -102,7 +102,7 @@ 
  uchar TableID(void) const { return tableID; } 
  uchar Version(void) const { return version; } 
  int RunningStatus(void) const { return runningStatus; } 
-  const char *Title(void) const { return title; } 
+  const char *Title(void) const { return title ? title : ""; } 
  const char *ShortText(void) const { return shortText; } 
  const char *Description(void) const { return description; } 
  const cComponents *Components(void) const { return components; }

Display More

SHF · March 9, 2025 at 10:31 PM

Quote from kls

Ich hatte für die Fehlersuche in cEvent::Title() und cEvent::SetTitle() jeweils eine Abfrage eingebaut, ob zu dem Zeitpunkt der jeweilige Thread einen Read- bzw. Write-Lock auf Schedules hat. Das war aber eine eher "krude" Angelegenheit, da ich dazu einen Pointer auf Schedules brauchte und Members von pthread_rwlock_t abfragen musste, die plattformabhängig sind (auf Raspberry Pi anders als auf Linux gcc). Das offiziell einzubauen würde ich gerne vermeiden.

Ich bin nicht ganz sicher ob das funktioniert, aber könnte man nicht einfach die Threadid des erstellenden Threads im Statekey-Objekt mit speichern.
Dann muss man nur prüfen, ob die Threadid des eigenen Thread in der Liste auftaucht und der Lock (read/write) stimmt, dann sollte alles o.k. sein.
An die Threadid des aktuellen Prozesses sollte man mit gettid() (oder besser pthread_self() ???) doch überall recht einfach ran kommen.

Eine einfache Möglichkeit die Locks zu prüfen wäre echt hilfreich.
Da jetzt Jahre nach Einführung (die Locks dürften doch auch bald 10Jähriges Jubiläum haben ) sogar im VDR noch fehlende gefunden werden, befürchte ich, dass sich bei den Plugins Abgründe auftun werden .

Quote from kls

dabei ist manchmal der Fall aufgetreten, dass ein cEvent entsteht, bei dem title==NULL ist, der aber anscheinend nicht durch FixEpgBugs() geht.

Das wäre echt blöd, wenn das sporadisch auch mit "sinnvollen" Events passiert, einige von der Korrekturen sind ja nicht nur Kosmetik. Amok laufende Suchtimer wären dann das geringste Problem, was mir so einfällt...
Irgendwie sehe aber auch keine ich Möglichkeit, wie sich ein Event an der Korrektur vorbei mogeln kann.

Das SetTitle() in der epg.c kann man wohl ausschließen, da es nur beim einlesen der epg.data verwendet wird. Dann würden die Fehler nur nach einem Neustart des VDR Auftreten, das hätte auffallen müssen.

Evtl. eine blöde Idee, aber ist wirklich sicher gestellt das FixEpgBugs() nicht aufgerufen wurde und dass das fragliche Event nie einen Titel hatte?
Nicht dass es eine Fehlfunktion in FixEpgBugs() ist, die durch irgendwelche komischen Sonderzeichen ausgelöst wird oder wenn der Titel nach FixEpgBugs() am Ende dann leer ist.

Wirklich gesehen habe ich da bislang nichts, aber könnte eine der Helper-Funktionen einen NULL-String zurück geben?

Schon versucht eine Abfrage auf title==NULL ganz am Ende von FixEpgBugs() einbauen?
Das wäre interessant, ob die anschlägt.

Ehrlich gesagt, verstehe ich trotz der Beschreibung hier noch immer nicht wirklich, was das für Events sind.
Mit ein Bisschen mehr Informationen könnte man vermutlich die Art des Fehlers deutlich eingrenzen.

Gibt es Gemeisamkeiten (außer dass der Titel fehlt )?
Trifft es immer den gleichen Kanal?
Sind das echte, sinnvolle Events, also Startzeit in den nächsten Wochen, plausible Länge, ...?

Existieren die Events wirklich im Schedule oder nur im Menü? Lassen sich zB. auch mit SVDRP ausgeben?
Wenn nicht, fehlt wohl immer noch irgendwo ein Lock.

Was passiert, wenn man das EPG mit CLRE komplett löscht und dann neu scannt? Tifft es dann die gleichen Everts erneut?

Tauchen die Fehler auch in der epg.data auf der Festplatte auf?
Wenn ich das richtig interpretiere, sollte da dann einfach der Titel fehlen.

Ich habe bei mir eben gesucht und nichts gefunden.
grep -zoP "\nE.+\n[^T]" ./epg.data (Liefert der Einfachheit halber nur die erste Zeile des Events!)
Im Produktiv-system ist aber eine recht alte VDR-Version am laufen (längere Geschichte).
Was, falls andere die Fehler finden, bedeuten müsste, dass der grundlegende Fehler erst später rein gekommen ist.

Falls jemand so ein Event in der epg.data findet, dann bitte hier mal rein stellen, gern auch mehrere. Ich würde es mir gerne mal ansehen.

Da das Problem anscheinend nicht viele haben und ORF erwähnt wurde, könnte es evtl. an der speziellen Emfangssituation liegen? An der Kombination DVB-S plus deutschem und österreichischem DVB-T, meine ich.

kls · March 9, 2025 at 11:44 PM

Ich habe das Gefühl, dass das, wenn überhaupt, bei obsoleten Kanälen auftritt. Irgendwas läuft da beim EPG anders als bei anderen.

Ich werde das für die nächste Version in cEvent so machen:

const char *Title(void) const { return title ? title : ""; }

MegaV0lt · March 10, 2025 at 10:01 AM

Quote from SHF

Ich habe bei mir eben gesucht und nichts gefunden.
grep -zoP "\nE.+\n[^T]" ./epg.data (Liefert der Einfachheit halber nur die erste Zeile des Events!)

Code

darkwing@vdr01:~/src/skin-flatplus/vdr-plugin-skinflatplus-1.0.7$ grep -zoP "\nE.+\n[^T]" /var/cache/vdr/epg.data

E 39470 1741595400 900 4E FF
edarkwing@vdr01:~/src/skin-flatplus/vdr-plugin-skinflatplus-1.0.7$

Man beachte das e zu beginn der letzten Zeile

kfb77 · March 10, 2025 at 10:24 AM

Genau das Gleiche auf meinem VDR:

root@VDR-2404-Dev:~# grep -zoP "\nE.+\n[^T]" /var/cache/vdr/epg.data

E 39470 1741595400 900 4E FF
eroot@VDR-2404-Dev:~#

Das ist der ganze Eintrag inklusive der davor und danach:

E 39062 1741593600 1800 4E 18
T Medienwerkstatt (Wh.)
S Dokus und Reportagen aus Nürnberg und der Region. Mehr Informationen finden Sie unter www.medienwerkstatt-franken.de
D Die MEDIENWERKSTATT Franken realisiert Dokumentationen und Reportagen über zeitgeschichtliche, kulturelle und soziale Themen aus Nürnberg und der Region. Ob Künstlerporträt, Stadtteil-Film, Milieu-Studie, Langzeitdokumentation oder ausführliches Interview - in ihren 30-minütigen Filmen berichtet die MEDIENWERKSTATT stets auf Augenhöhe und lässt Menschen zu Wort kommen, die eher selten Gehör finden. Die Sendung läuft jeden Sonntag um 19, 21 und 22 Uhr, sowie montags, mittwochs und freitags in Wiederholungen. Mehr Infos unter medienwerkstatt-franken.de.
e
E 39470 1741595400 900 4E FF
e
E 39471 1741596300 1800 4E 1D
T Medienwerkstatt (Wh.)
S Dokus und Reportagen aus Nürnberg und der Region. Mehr Informationen finden Sie unter www.medienwerkstatt-franken.de
D Die MEDIENWERKSTATT Franken realisiert Dokumentationen und Reportagen über zeitgeschichtliche, kulturelle und soziale Themen aus Nürnberg und der Region. Ob Künstlerporträt, Stadtteil-Film, Milieu-Studie, Langzeitdokumentation oder ausführliches Interview - in ihren 30-minütigen Filmen berichtet die MEDIENWERKSTATT stets auf Augenhöhe und lässt Menschen zu Wort kommen, die eher selten Gehör finden. Die Sendung läuft jeden Sonntag um 19, 21 und 22 Uhr, sowie montags, mittwochs und freitags in Wiederholungen. Mehr Infos unter medienwerkstatt-franken.de.
e

kls · March 10, 2025 at 10:43 AM

Das ist anscheinend ein Event ohne jede Descriptoren. Aber selbst der der müsste in eit.c durch FixEpgBugs() gehen bzw. in cEvent::Read() der Title gesetzt werden.

Absturz wegen cEvent::title == NULL - Warum???

Participate now!

Users Viewing This Thread