Compile Warnings in vnsiserver

  • Verwendet jemand zufällig das SAT>IP-Plugin in Verbindung mit ShowChannelNamesWithSource == 2 (Das ist mit Anzeige der SAT-Position.)?

    Nur um sicher zu stellen, dass es nicht daran liegt.

    Zum Testen sollte es reichen das zu aktivieren und die ein- zwei-mal die Kanalliste durch zu scrollen.

    Mit einer DVB-Karte werde ich es die Tage mal testen, aber mit SAT>IP kann ich es leider nicht.


    Ich habe das eben mal eingestellt, habe aber nur 56 Kanäle in der channels.conf.

    Ich konnte jedoch keinen crash provozieren.

    Gentoo Linux ~ VDR 2.6.6 ~ DD Octopus NET V2 S2 Max - SAT>IP ~ LENOVO ThinkServer TS200V ~ Intel(R) Core(TM) i5 CPU680@3.60GHz ~ 16GB RAM ~ NVIDIA T400

  • Wenn es satip ist, dann müsste die sources.conf editiert worden sein, um die Auswahl des Satelliten festzulegen. Wäre also auch mal interessant, die Datei zu sehen.



    Aber da wir immer über cChannel::Name stolpern:




    Was ich auch nicht so ganz verstehe, ist die Bedeutung der Variable nameSourceMode in vdr/channels.c.

    Diese Variable wird niemals initialisiert im Konstruktor, aber wird direkt in cChannel::Name verwendet. Ich hätte im Konstruktor erwartet

    Code
    nameSourceMode = Setup.ShowChannelNamesWithSource;


    Wenn nameSourceMode zugewiesen wird, wird immer der gleiche Wert '0' zugewiesen.

    Diese Variable ist also entweder uninitiatlisiert, oder gleich '0'. Kein anderer Wert.


    Ich hätte ja auch erwartet, dass diese Variable dann nach diesem Vergleich in channels.c:110 neu gesetzt wird:

    Code
         if (isempty(nameSource) || nameSourceMode != Setup.ShowChannelNamesWithSource) {
    ++      nameSourceMode = Setup.ShowChannelNamesWithSource;
            if (Setup.ShowChannelNamesWithSource == 1)
               nameSource = cString::sprintf("%s (%c)", name, cSource::ToChar(source));
            else
               nameSource = cString::sprintf("%s (%s)", name, *cSource::ToString(source));
            }


    Auch in cChannel::operator= wird der variable 0 zugewiesen, anstelle vom rhs Channel.nameSourceMode


    In cChannel::SetTransponderData wird dann nameSourceMode auf 0 gesetzt (warum nicht Setup.ShowChannelNamesWithSource?), wenn

    Code
    if (source != Source || frequency != Frequency || srate != Srate || strcmp(parameters, Parameters)) {

    Dabei lohnt sich das nur, wenn source != Source.. Jede einzelne Änderung an den Transponderdaten bewirkt Neuzweisung von nameSource, obwohl das gar nicht nötig wäre.


    Hat jemand ne Idee?

  • Quote

    RHS Das wäre eventuell hilfreich, wenn Du die noch finden könntest.

    Da man an den Punkt eigentlich nie gelangen sollte, könnten die Informationen Aufschluss geben, was da schief läuft.

    Ich finde diese Fehlermeldung nicht.

    wirbel: Ich habe die zeile 2 wie oben beschrieben eingefügt und ShowChannelNamesWithSource == 2 gesetzt.


    dann erhalte ich folgenden anderen verlauf im Syslog beim Channel Switch und bisher kein crash. Bisher hätte es wohl einen gegeben.

    Interessant ist :

    [10235] VNSI: exit streamer thread

    während des Channel switches. Hatte ich bisher nie.


  • wirbel


    good pick.


    Code
         if (isempty(nameSource) || nameSourceMode != Setup.ShowChannelNamesWithSource) {
    ++      nameSourceMode = Setup.ShowChannelNamesWithSource;
            if (Setup.ShowChannelNamesWithSource == 1)
               nameSource = cString::sprintf("%s (%c)", name, cSource::ToChar(source));
            else
               nameSource = cString::sprintf("%s (%s)", name, *cSource::ToString(source));
            }


    scheint die Verbesserung gebracht zu haben kein crash bisher mit drei kodi clients und häufigen umschalten.

  • Jede einzelne Änderung an den Transponderdaten bewirkt Neuzweisung von nameSource, obwohl das gar nicht nötig wäre.

    Hat jemand ne Idee?

    Der Knackpunkt müsste aber sein, dass wir was am cCannel-Objekt ändern, obwohl wir nur einen readlock darauf haben.

    Gruss
    SHF


  • Egal ob mit oder ohne den Patch, die einfache Abfrage von cChannel::Name() ändert in jedem Fall cChannel::nameSource und ist damit immer ein Schreibzugriff.

    Und noch schlimmer - das bei jedem Zugriff auf cChannel::Name(), da die if Bedingung


    Code
    if (isempty(nameSource) || nameSourceMode != Setup.ShowChannelNamesWithSource) {

    niemals falsch ergeben kann. nameSourceMode wird immer '0' bleiben und Setup.ShowChannelNamesWithSource bleibt immer bei dem was der Nutzer einstellt, also hier '2'.



    Es fehlt also in jedem Fall ein Lock oder ähnliches. Aber dafür müsste nur cChannel gelockt werden, nicht die ganze Kanalliste.

  • Ich denke mal, dass das hier fehlt:

    Diff
    --- channels.c  2021/05/21 09:38:34     5.1
    +++ channels.c  2024/03/01 07:22:19
    @@ -112,6 +112,7 @@
                nameSource = cString::sprintf("%s (%c)", name, cSource::ToChar(source));
             else
                nameSource = cString::sprintf("%s (%s)", name, *cSource::ToString(source));
    +        nameSourceMode = Setup.ShowChannelNamesWithSource;
             }
          return nameSource;
          }
  • Wird zusätzlich ein Lock benötigt? Verändert ja cChannel, evtl. von mehreren Threads aus.

  • Vielleicht so (ungetestet!):

  • Gut wäre diese Zeile in vdr/channels.c:56ff noch hinzuzufügen.

    Ansonsten hat die Variable u.U. einen beliebigen Startwert.


  • kls


    Sorry aber es wäre wohl mittlerweile sinnvoll den Patchstatus für vdr und vnsiserver festzulegen um eine transparente Testbasis zu haben. Mir erschließen sich die Abhängigkeiten der verschiedenen Vorschläge jetzt nicht mehr so ganz.

  • Hallo RHS,


    ja, das macht definitiv Sinn, Klarheit über die Teststände ist essentiell.

    Aber ohne deine intensiven Tests gäbe es keinen Fortschritt bei vnsi, danke dafür.

    Ich packe das mal in zwei temporäre github repos auf meinem Account, das macht es leichter für dich.


    ------------------------------------------------------------------------------------

    für vnsiserver bitte das hier verwenden:

    GitHub - wirbel-at-vdr-portal/vdr-plugin-vnsiserver-crashes: VDR plugin to handle XBMC clients.
    VDR plugin to handle XBMC clients. Contribute to wirbel-at-vdr-portal/vdr-plugin-vnsiserver-crashes development by creating an account on GitHub.
    github.com

    git clone https://github.com/wirbel-at-v…in-vnsiserver-crashes.git


    identisch zu:

    Commits · vdr-projects/vdr-plugin-vnsiserver
    VDR plugin to handle XBMC clients. Contribute to vdr-projects/vdr-plugin-vnsiserver development by creating an account on GitHub.
    github.com

    + processChannels_GetChannels_StateKey.diff

    -----------------------------------------------------------------------------------


    -----------------------------------------------------------------------------------

    für VDR bitte das hier verwenden:

    GitHub - wirbel-at-vdr-portal/vdr-cChannel-Name: temporary git repo for debugging cChannel::Name()
    temporary git repo for debugging cChannel::Name(). Contribute to wirbel-at-vdr-portal/vdr-cChannel-Name development by creating an account on GitHub.
    github.com

    git clone https://github.com/wirbel-at-v…tal/vdr-cChannel-Name.git


    identisch zu:

    vdr-2.6.6 (ohne Patches)

    + changes in cChannels.c for cChannel::Name() (wirbel + kls)

    -----------------------------------------------------------------------------------


    Du kannst aus beiden repos auch einfach die letzten commits als Patche downloaden..

  • M-Reimer - danke! Sehe ich ganz genau so, ich hatte das schon gar nicht mehr auf dem Schirm.

    Dieser Patch von MarkusE+SHF sollte auf jeden Fall in die nächste VDR Version.


    Habe ich in mein temporäres git aufgenommen.

    Um so mehr hat RHS mit seinem Kommentar/Nachfrage recht..

  • Ich denke, die dynamische Generierung von nameSource (und shortNameSource) generell problematisch.

    (Sorry, ich hätte das wohl etwas ausführlicher schreiben sollen, war aber gestern etwas in Eile.)

    (Btw.: Danke an heifisch fürs testen! )


    So wie ich es verstanden habe, sind die cChannel einmalig und quasi statisch (sonst würde das nameSourceMode ja keinen Sinn machen).

    Solange ich einen einen Readlock auf der Kanallist habe, muss ich mich darauf verlassen können, dass sich in der Liste nichts ändert.

    Bei der Generierung von nameSource ändere ich aber (wahrscheinlich) auch den Pointer darauf. Sobald mehrere Threads gleichzeitig auf die Kanalliste zugreifen besteht da immer die Chance, dass da einer einen ungültigen Pointer bekommt.

    Und ich denke, das passiert hier auch gerade.


    Die einzige Möglichkeit, wie man das wirklich sauber lösen kann, ist die Generierung von nameSource in einem Writelock auszuführen. Das geht natürlich nicht, solange ein Readlock existiert, man müsste das also in eine eigene Funktion auslagern und bei Bedarf aufrufen.

    Das wäre dann bei erstellen den cChannel-Objekts und bei Änderung von Setup.ShowChannelNamesWithSource.

    Letzteres passiert aber wohl nur im Menü durch den Nutzer? Wäre also überschaubar.


    Insgesamt müsste man wohl etwas mehr umbauen, befürchte ich, es ist aber zumindest einzige Möglichkeit, das wirklich zu lösen, die mir einfällt.

    Gruss
    SHF


Participate now!

Don’t have an account yet? Register yourself now and be a part of our community!