Memleak in movieplayer_menu.cpp

[Gaucho316]

In Zeile 96 von movieplayer_menu.cpp wird ja das Objekt moviePlayerGui erzeugt. Muss da nicht ein if drumrum, damit es nicht immer wieder neu erzeugt wird?

Code: Alles auswählen

if (moviePlayerGui == NULL)
	moviePlayerGui = new CMoviePlayerGui();

[dbt]

Sollte eigentlich im Constructor/Destructor geregelt sein.

[Gaucho316]

Richtig, aber ist es nicht so, dass das in Zeile 214 von neutrino_menu.cpp instanzierte Object "new CMoviePlayerMenue()" während der Laufzeit von Neutrino im Speicher bleibt und somit der Destructor nie aufgerufen wird und durch Aufruf von "new CMoviePlayerMenue()"->exec(...) moviePlayerGui immer wieder neu erzeugt wird?

[seife]

Ich denke das passt schon, der code ist zwar etwas durcheinander, was die stellen angeht wo das new und das delete gemacht wird, aber ich vermute, dass das schon passt.

Zum probieren kannst du ja mal vor dem "new" und vor dem "delete" (im destruktor, Zeile 70) ein printf reinmachen und schauen, ob das schön symmetrisch aufgerufen wird.

Ansonsten schadet die Abfrage sicher nicht, aber bitte dann auch in Zeile 71 ein moviePlayerGui = NULL machen und es dafür aus der Zeile 57 löschen.

Hm. Ob der Destruktor tatsächlich aufgerufen wird, kann ich jetzt so aus dem Kopf auch nicht sagen, und die Neutrino-GUI-Klassen sind die Hölle, da schau ich jetzt nicht rein

[dbt]

Ich glaube der Destructor wird nur aufgerufen wenn das Hauptemenü durchgelaufen ist. Das wird aber irgendwie in der Widgetklasse gemacht, habs aber da noch nie so genau ausgemacht. Würde das beferiedigender aussehen:

Code: Alles auswählen

diff --git a/tuxbox/neutrino/src/gui/movieplayer_menu.cpp b/tuxbox/neutrino/src/gui/movieplayer_menu.cpp
index 9790e8b..fcf995c 100644
--- a/tuxbox/neutrino/src/gui/movieplayer_menu.cpp
+++ b/tuxbox/neutrino/src/gui/movieplayer_menu.cpp
@@ -54,8 +54,6 @@ CMoviePlayerMenue::CMoviePlayerMenue()
 {
 	frameBuffer = CFrameBuffer::getInstance();
 
-	moviePlayerGui = NULL;
-
 	width = w_max (500, 100);
 	hheight = g_Font[SNeutrinoSettings::FONT_TYPE_MENU_TITLE]->getHeight();
 	mheight = g_Font[SNeutrinoSettings::FONT_TYPE_MENU]->getHeight();
@@ -67,7 +65,7 @@ CMoviePlayerMenue::CMoviePlayerMenue()
 
 CMoviePlayerMenue::~CMoviePlayerMenue()
 {
-	delete moviePlayerGui;
+
 }
 
 int CMoviePlayerMenue::exec(CMenuTarget* parent, const std::string &/*actionKey*/)
@@ -93,7 +91,7 @@ void CMoviePlayerMenue::hide()
 
 void CMoviePlayerMenue::showMoviePlayerMenue()
 {
-	moviePlayerGui = new CMoviePlayerGui();
+	CMenuTarget* 	moviePlayerGui = new CMoviePlayerGui();
 
 	//init
 	CMenuWidget * mpmenue = new CMenuWidget(LOCALE_MAINMENU_MOVIEPLAYER, NEUTRINO_ICON_EPGINFO, width);
@@ -103,15 +101,15 @@ void CMoviePlayerMenue::showMoviePlayerMenue()
 	mpmenue->addItem(GenericMenuSeparatorLine);
 
 	//ts playback 
-	mpmenue->addItem(new CMenuForwarder(LOCALE_MOVIEPLAYER_TSPLAYBACK, true, NULL, this->moviePlayerGui, "tsplayback", CRCInput::RC_green, NEUTRINO_ICON_BUTTON_GREEN));
+	mpmenue->addItem(new CMenuForwarder(LOCALE_MOVIEPLAYER_TSPLAYBACK, true, NULL, moviePlayerGui, "tsplayback", CRCInput::RC_green, NEUTRINO_ICON_BUTTON_GREEN));
 	//ts playback pin 
-	mpmenue->addItem(new CMenuForwarder(LOCALE_MOVIEPLAYER_TSPLAYBACK_PC, true, NULL, this->moviePlayerGui, "tsplayback_pc", CRCInput::RC_1));
+	mpmenue->addItem(new CMenuForwarder(LOCALE_MOVIEPLAYER_TSPLAYBACK_PC, true, NULL, moviePlayerGui, "tsplayback_pc", CRCInput::RC_1));
 
 	neutrino_msg_t rc_msg;
 #ifdef ENABLE_MOVIEBROWSER
 #ifndef ENABLE_MOVIEPLAYER2
 	//moviebrowser init via movieplayer 1
-	mpmenue->addItem(new CMenuForwarder(LOCALE_MOVIEBROWSER_HEAD, true, NULL, this->moviePlayerGui, "tsmoviebrowser", CRCInput::RC_2));
+	mpmenue->addItem(new CMenuForwarder(LOCALE_MOVIEBROWSER_HEAD, true, NULL, moviePlayerGui, "tsmoviebrowser", CRCInput::RC_2));
 #else
 	//moviebrowser init
 	mpmenue->addItem(new CMenuForwarder(LOCALE_MOVIEBROWSER_HEAD, true, NULL, CMovieBrowser::getInstance(), "run", CRCInput::RC_2));
@@ -122,16 +120,16 @@ void CMoviePlayerMenue::showMoviePlayerMenue()
 #endif /* ENABLE_MOVIEBROWSER */
 
 	//bookmark
-	mpmenue->addItem(new CMenuForwarder(LOCALE_MOVIEPLAYER_BOOKMARK, true, NULL, this->moviePlayerGui, "bookmarkplayback", rc_msg));
+	mpmenue->addItem(new CMenuForwarder(LOCALE_MOVIEPLAYER_BOOKMARK, true, NULL, moviePlayerGui, "bookmarkplayback", rc_msg));
 
 	mpmenue->addItem(GenericMenuSeparatorLine);
 
 	//vlc file play
-	mpmenue->addItem(new CMenuForwarder(LOCALE_MOVIEPLAYER_FILEPLAYBACK, true, NULL, this->moviePlayerGui, "fileplayback", CRCInput::RC_red, NEUTRINO_ICON_BUTTON_RED));
+	mpmenue->addItem(new CMenuForwarder(LOCALE_MOVIEPLAYER_FILEPLAYBACK, true, NULL, moviePlayerGui, "fileplayback", CRCInput::RC_red, NEUTRINO_ICON_BUTTON_RED));
 	//vlc dvd play
-	mpmenue->addItem(new CMenuForwarder(LOCALE_MOVIEPLAYER_DVDPLAYBACK, true, NULL, this->moviePlayerGui, "dvdplayback", CRCInput::RC_yellow, NEUTRINO_ICON_BUTTON_YELLOW));
+	mpmenue->addItem(new CMenuForwarder(LOCALE_MOVIEPLAYER_DVDPLAYBACK, true, NULL, moviePlayerGui, "dvdplayback", CRCInput::RC_yellow, NEUTRINO_ICON_BUTTON_YELLOW));
 	//vlc vcd play
-	mpmenue->addItem(new CMenuForwarder(LOCALE_MOVIEPLAYER_VCDPLAYBACK, true, NULL, this->moviePlayerGui, "vcdplayback", CRCInput::RC_blue, NEUTRINO_ICON_BUTTON_BLUE));
+	mpmenue->addItem(new CMenuForwarder(LOCALE_MOVIEPLAYER_VCDPLAYBACK, true, NULL, moviePlayerGui, "vcdplayback", CRCInput::RC_blue, NEUTRINO_ICON_BUTTON_BLUE));
 
 	mpmenue->addItem(GenericMenuSeparatorLine);
 
@@ -146,5 +144,6 @@ void CMoviePlayerMenue::showMoviePlayerMenue()
 	mpmenue->exec(NULL, "");
 	mpmenue->hide();
 	delete mpmenue;
+	
 }
 
diff --git a/tuxbox/neutrino/src/gui/movieplayer_menu.h b/tuxbox/neutrino/src/gui/movieplayer_menu.h
index fc0dc8a..468b8ee 100644
--- a/tuxbox/neutrino/src/gui/movieplayer_menu.h
+++ b/tuxbox/neutrino/src/gui/movieplayer_menu.h
@@ -49,8 +49,6 @@ class CMoviePlayerMenue : public CMenuTarget
 		void hide();
 		void showMoviePlayerMenue();
 
-		CMenuTarget* 			moviePlayerGui;
-
 	public:	
 		CMoviePlayerMenue();
 		~CMoviePlayerMenue();

[Gaucho316]

Fehlt da nicht delete moviePlayerGui?

[dbt]

Ja, habs grad mal probiert. Mit dem delete gibts mal kurz einen Aussetzeter und ohne das log dazu gesehen zu haben, denke ich, dass es da hingehört. Ich dachte eigentlich dass die Targets immer intern aufgelöst werden. Das müsste man sich auch mal ansehen ...Teste das bitte mal ...

[Gaucho316]

Ich dachte eigentlich dass die Targets immer intern aufgelöst werden.

Hast Recht. Ich habe mir eben mal den Destructor von CMenuWidget angesehen. Dort werden die übergebenen Menü-Elemente gelöscht. Deshalb produziert der aktuelle CVS-Stand an der Stelle doch kein Memleak. Dein geposteter Code ist trotzdem besser.

[dbt]

Denke ich auch, also rein oder?

[Gaucho316]

Na klar.