[PATCH] CCam()-Class fd leak wenn zapit in standby geht

[seife]

0001-close-the-socket-fd-when-deleting-CCam-objects.patch

Wenn man zapit in standby schickt und wieder aufweckt (movieplayer oder "pzapit -esb; pzapit -lsb"), dann leaked es einen socked Filedescriptor. Der geht verloren, weil das CCam()-Objekt nicht hinter sich aufräumt.

Dieser relativ triviale Patch fixed das für mich, aber da ich kein C++-Spezi bin (das "virtual" vor dem Destruktor habe ich wegen der Compilerwarnings hingeschrieben... ) wäre es vermutlich gut, wenn sich das noch jemand kurz anschaut.
Alternativ könnte man auch in CCam::sendMessage() den fd sofort wieder schliessen, denke ich.

[Striper]

Wenn man zapit in standby schickt und wieder aufweckt (movieplayer oder "pzapit -esb; pzapit -lsb"), dann leaked es einen socked Filedescriptor. Der geht verloren, weil das CCam()-Objekt nicht hinter sich aufräumt.

Könntest du das bitte etwas genauer erklären? Ich konnte hier nie ein Problem feststellen. Wie kann ich das nachstellen, bzw. was genau passiert da? ^^

[seife]

Code: Alles auswählen

/var $ pidof zapit
174
/var $ ls -l /proc/174/fd |grep socket
lrwx------    1 root     root           64 Feb 22 16:00 13 -> socket:[1755]
lrwx------    1 root     root           64 Feb 22 16:00 4 -> socket:[251]
/var $ pzapit -esb; pzapit -lsb
/var $ ls -l /proc/174/fd |grep socket
lrwx------    1 root     root           64 Feb 22 16:01 13 -> socket:[1755]
lrwx------    1 root     root           64 Feb 22 16:01 14 -> socket:[2285]
lrwx------    1 root     root           64 Feb 22 16:01 4 -> socket:[251]
/var $ pzapit -esb; pzapit -lsb
/var $ ls -l /proc/174/fd |grep socket
lrwx------    1 root     root           64 Feb 22 16:01 13 -> socket:[1755]
lrwx------    1 root     root           64 Feb 22 16:01 14 -> socket:[2285]
lrwx------    1 root     root           64 Feb 22 16:01 15 -> socket:[2441]
lrwx------    1 root     root           64 Feb 22 16:01 4 -> socket:[251]
/var $

Mit jedem standby wird es ein socket-FD mehr. Bei 1024 offenen Filedeskriptoren dürfte dann Schluss sein

[Striper]

:dash:

So, nu habs auch ich kapiert. Werds mal einbauen und testen.

[seife]

Die Frage ist, ob das korrekt ist. Es funktioniert, das steht ausser Frage

[GetAway]

Ich kann das hier nicht nachvollziehen. Gibts da eventuell noch eine Besonderheit? Sprich: nur bei Sat?

[seife]

Eigentlich nicht. Evtl. tritt es nur auf, wenn man auch tatsächlich irgendeinen camd laufen hat (ich habe normalerweise nicht mal den camd2, weil ich kein Premiere habe). Wenn die Verbindung zum Camd gleich fehlschlägt, dann lungert vermutlich später auch kein socket-FD rum.

[rhabarber1848]

Ich kann seifes Beobachtungen und die Funktionsfähigkeit von seifes Patch bestätigen:

CVS-Code:

Code: Alles auswählen

/var # ls -l /proc/`pidof zapit`/fd |grep socket
lrwx------    1 root     root           64 Feb 28 20:21 13 -> socket:[25178]
lrwx------    1 root     root           64 Feb 28 20:21 4 -> socket:[309]
/var # pzapit -esb; pzapit -lsb
/var # ls -l /proc/`pidof zapit`/fd |grep socket
lrwx------    1 root     root           64 Feb 28 20:21 13 -> socket:[25178]
lrwx------    1 root     root           64 Feb 28 20:21 15 -> socket:[25882]
lrwx------    1 root     root           64 Feb 28 20:21 4 -> socket:[309]
/var # pzapit -esb; pzapit -lsb
/var # ls -l /proc/`pidof zapit`/fd |grep socket
lrwx------    1 root     root           64 Feb 28 20:21 13 -> socket:[25178]
lrwx------    1 root     root           64 Feb 28 20:21 15 -> socket:[25882]
lrwx------    1 root     root           64 Feb 28 20:21 16 -> socket:[26211]
lrwx------    1 root     root           64 Feb 28 20:21 4 -> socket:[309]
/var # pzapit -esb; pzapit -lsb
/var # ls -l /proc/`pidof zapit`/fd |grep socket
lrwx------    1 root     root           64 Feb 28 20:22 13 -> socket:[25178]
lrwx------    1 root     root           64 Feb 28 20:22 15 -> socket:[25882]
lrwx------    1 root     root           64 Feb 28 20:22 16 -> socket:[26211]
lrwx------    1 root     root           64 Feb 28 20:22 17 -> socket:[26547]
lrwx------    1 root     root           64 Feb 28 20:22 4 -> socket:[309]
# 

Gepatcht:

Code: Alles auswählen

# ls -l /proc/`pidof zapit`/fd |grep socket
lrwx------    1 root     root           64 Feb 28 20:29 13 -> socket:[745]
lrwx------    1 root     root           64 Feb 28 20:29 4 -> socket:[238]
# pzapit -esb; pzapit -lsb
# ls -l /proc/`pidof zapit`/fd |grep socket
lrwx------    1 root     root           64 Feb 28 20:29 12 -> socket:[1122]
lrwx------    1 root     root           64 Feb 28 20:29 4 -> socket:[238]
# pzapit -esb; pzapit -lsb
# ls -l /proc/`pidof zapit`/fd |grep socket
lrwx------    1 root     root           64 Feb 28 20:29 12 -> socket:[1460]
lrwx------    1 root     root           64 Feb 28 20:29 4 -> socket:[238]
# pzapit -esb; pzapit -lsb
# ls -l /proc/`pidof zapit`/fd |grep socket
lrwx------    1 root     root           64 Feb 28 20:30 12 -> socket:[1798]
lrwx------    1 root     root           64 Feb 28 20:30 4 -> socket:[238]
#

[GetAway]

Evtl. tritt es nur auf, wenn man auch tatsächlich irgendeinen camd laufen hat

Stimmt, mit aktiver camd2 kann ich jetzt auch nachvollziehen. Ohne bleibts unverändert.

[rhabarber1848]

@seife: Auf der dunklen Seite ist dieser Code, der für ein anderes
Thema geschrieben wurde, irgendwann mal aufgetaucht:

Code: Alles auswählen

--- a/apps/dvb/zapit/include/zapit/cam.h        9 Feb 2003 19:22:08 -0000       1.25
+++ b/apps/dvb/zapit/include/zapit/cam.h        2 Jun 2008 15:27:18 -0000
@@ -35,6 +35,8 @@ class CCam : public CBasicClient

        public:
                bool setCaPmt(CCaPmt * const caPmt);
+               unsigned int number;
+               CCam(int Number);
 };

 #endif /* __cam_h__ */
--- a/apps/dvb/zapit/src/zapci/cam.cpp  4 Apr 2004 20:20:45 -0000       1.33
+++ b/apps/dvb/zapit/src/zapci/cam.cpp  2 Jun 2008 15:27:23 -0000
@@ -31,9 +31,21 @@ const unsigned char CCam::getVersion(voi
        return 0x9F;
 }

+CCam::CCam(int Number)
+{
+       number = Number;
+}
+
 const char *CCam::getSocketName(void) const
 {

Wahrscheinlich muss der Code von Hand eingebaut werden, da ich
ihn aus einem größeren Code-Fragment ausgeschnitten habe.
Ob der Code den C++-Standards nun besser entspricht, kann ich
leider mangels Kenntnissen nicht beurteilen. Ich poste ihn nur als
Diskussionsgrundlage, Quell-URL gibt es nur gegen PM, allerdings
nur, sofern das nicht gegen die Boardregeln verstösst.

[seife]

Dieser Code macht was anderes.
Das Problem ist, dass es keinen Destruktor gibt, der den fd wieser schliesst.

Was du suchen musst ist nicht eine Funktion
CCam::CCam() => das ist der Konstruktor, sondern
CCam::~CCam(), weil die ist der Destruktor. Wenn es den dort nicht gibt, dann hat dieser code dasselbe Problem.

Ansonsten ist das nun wahrlich kein Geheimnis, was der Rest dieses Codefragments macht, und mehrere camds anzusteuern ist ja wirklich trivial Und dafür hat es halt der default-Konstruktor nicht getan, deswegen wurde da ein eigener gemacht.

Wie gesagt: Ich weiss, dass mein Destruktor funktioniert, allerdings bin ich mir nicht sicher wegen dem ganzen "virtual"-Gedöns etc.
Egal, ich frag nächste Woche mal nen Kollegen, der sich "etwas" besser als ich in C++ auskennt.

[rhabarber1848]

CCam::CCam() => das ist der Konstruktor, sondern
CCam::~CCam(), weil die ist der Destruktor.

Danke, wieder was gelernt

[rhabarber1848]

Dieser Patch ist Bestandteil des control_zapit_merge-Branches,
wenn er keine Schwierigkeiten gemacht hat, checke ich ihn ein.

[seife]

Ich habe bisher keine negativen Nebenwirkungen bemerkt.

[GetAway]

Dito

[rhabarber1848]

committed to CVS