widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1776603_map_info_memory_leak into lp:widelands.

Commit message:
Added cleanup function to map_info.cc and initializing the sound system. This gets rid of some error messages and a memory leak. Pulled out common code for the website binaries.


Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1776603 in widelands: "ASAN: wl_map_object_info leaks memory"
  https://bugs.launchpad.net/widelands/+bug/1776603

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1776603_map_info_memory_leak/+merge/353140
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1776603_map_info_memory_leak into lp:widelands.

=== modified file 'src/graphic/gl/initialize.cc'
--- src/graphic/gl/initialize.cc	2018-04-07 16:59:00 +0000
+++ src/graphic/gl/initialize.cc	2018-08-15 03:33:41 +0000
@@ -84,6 +84,8 @@
 	// See graphic/gl/system_headers.h for an explanation of the next line.
 	glewExperimental = GL_TRUE;
 	GLenum err = glewInit();
+	// LeakSanitizer reports a memory leak which is triggered somewhere above this line, probably coming from the gaphics drivers
+
 	if (err != GLEW_OK) {
 		log("glewInit returns %i\nYour OpenGL installation must be __very__ broken. %s\n", err,
 		    glewGetErrorString(err));

=== modified file 'src/graphic/graphic.cc'
--- src/graphic/graphic.cc	2018-06-01 08:54:25 +0000
+++ src/graphic/graphic.cc	2018-08-15 03:33:41 +0000
@@ -85,6 +85,7 @@
 	                    window_mode_width_, window_mode_height_, SDL_WINDOW_OPENGL);
 
 	GLint max;
+	// LeakSanitizer reports a memory leak which is triggered somewhere in this function call, probably coming from the gaphics drivers
 	gl_context_ = Gl::initialize(
 	   trace_gl == TraceGl::kYes ? Gl::Trace::kYes : Gl::Trace::kNo, sdl_window_, &max);
 

=== modified file 'src/website/CMakeLists.txt'
--- src/website/CMakeLists.txt	2017-11-20 13:50:51 +0000
+++ src/website/CMakeLists.txt	2018-08-15 03:33:41 +0000
@@ -1,7 +1,20 @@
+wl_library(website_common
+  SRCS
+    website_common.cc
+    website_common.h
+  USES_SDL2
+  DEPENDS
+    base_exceptions
+    base_i18n
+    graphic
+    io_filesystem
+    sound
+)
+
+
 wl_binary(wl_map_info
   SRCS
     map_info.cc
-  USES_SDL2
   DEPENDS
     base_log
     graphic
@@ -12,6 +25,7 @@
     io_filesystem
     logic
     map_io_map_loader
+    website_common
 )
 
 wl_binary(wl_map_object_info
@@ -19,7 +33,6 @@
     map_object_info.cc
   USES_SDL2
   DEPENDS
-    base_i18n
     base_log
     base_macros
     graphic
@@ -27,5 +40,5 @@
     io_filesystem
     logic
     logic_tribe_basic_info
-    sound
+    website_common
 )

=== modified file 'src/website/map_info.cc'
--- src/website/map_info.cc	2018-04-07 16:59:00 +0000
+++ src/website/map_info.cc	2018-08-15 03:33:41 +0000
@@ -36,26 +36,10 @@
 #include "logic/editor_game_base.h"
 #include "logic/map.h"
 #include "map_io/widelands_map_loader.h"
+#include "website/website_common.h"
 
 using namespace Widelands;
 
-namespace {
-
-// Setup the static objects Widelands needs to operate and initializes systems.
-void initialize() {
-	if (SDL_Init(SDL_INIT_VIDEO) != 0) {
-		throw wexception("Unable to initialize SDL: %s", SDL_GetError());
-	}
-
-	g_fs = new LayeredFileSystem();
-	g_fs->add_file_system(&FileSystem::create(INSTALL_DATADIR));
-
-	g_gr = new Graphic();
-	g_gr->initialize(Graphic::TraceGl::kNo, 1, 1, false);
-}
-
-}  // namespace
-
 int main(int argc, char** argv) {
 	if (!(2 <= argc && argc <= 3)) {
 		log("Usage: %s <map file>\n", argv[0]);
@@ -142,9 +126,12 @@
 			write_string("}\n");
 			fw.write(*in_out_filesystem, (map_file + ".json").c_str());
 		}
+		egbase.cleanup_objects();
 	} catch (std::exception& e) {
 		log("Exception: %s.\n", e.what());
+		cleanup();
 		return 1;
 	}
+	cleanup();
 	return 0;
 }

=== modified file 'src/website/map_object_info.cc'
--- src/website/map_object_info.cc	2018-04-07 16:59:00 +0000
+++ src/website/map_object_info.cc	2018-08-15 03:33:41 +0000
@@ -24,7 +24,6 @@
 #include <boost/format.hpp>
 #include <boost/lexical_cast.hpp>
 
-#include "base/i18n.h"
 #include "base/log.h"
 #include "base/macros.h"
 #include "config.h"
@@ -36,7 +35,7 @@
 #include "logic/map_objects/tribes/tribe_basic_info.h"
 #include "logic/map_objects/tribes/tribes.h"
 #include "logic/map_objects/world/world.h"
-#include "sound/sound_handler.h"
+#include "website/website_common.h"
 
 using namespace Widelands;
 
@@ -44,52 +43,6 @@
 
 /*
  ==========================================================
- SETUP
- ==========================================================
- */
-
-// Setup the static objects Widelands needs to operate and initializes systems.
-std::unique_ptr<FileSystem> initialize(const std::string& output_path) {
-	i18n::set_locale("en");
-
-	if (SDL_Init(SDL_INIT_VIDEO) != 0) {
-		throw wexception("Unable to initialize SDL: %s", SDL_GetError());
-	}
-
-	g_fs = new LayeredFileSystem();
-	g_fs->add_file_system(&FileSystem::create(INSTALL_DATADIR));
-
-	std::unique_ptr<FileSystem> out_filesystem(&FileSystem::create(output_path));
-
-	// We don't really need graphics or sound here, but we will get error messages
-	// when they aren't initialized
-	g_gr = new Graphic();
-	g_gr->initialize(Graphic::TraceGl::kNo, 1, 1, false);
-
-	g_sound_handler.init();
-	g_sound_handler.nosound_ = true;
-	return out_filesystem;
-}
-
-// Cleanup before program end
-void cleanup() {
-	g_sound_handler.shutdown();
-
-	if (g_gr) {
-		delete g_gr;
-		g_gr = nullptr;
-	}
-
-	if (g_fs) {
-		delete g_fs;
-		g_fs = nullptr;
-	}
-
-	SDL_Quit();
-}
-
-/*
- ==========================================================
  SPECIALIZED FILEWRITE
  ==========================================================
  */
@@ -530,7 +483,8 @@
 	const std::string output_path = argv[argc - 1];
 
 	try {
-		std::unique_ptr<FileSystem> out_filesystem = initialize(output_path);
+		initialize();
+		std::unique_ptr<FileSystem> out_filesystem(&FileSystem::create(output_path));
 		EditorGameBase egbase(nullptr);
 		write_tribes(egbase, out_filesystem.get());
 	} catch (std::exception& e) {

=== added file 'src/website/website_common.cc'
--- src/website/website_common.cc	1970-01-01 00:00:00 +0000
+++ src/website/website_common.cc	2018-08-15 03:33:41 +0000
@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2018 by the Widelands Development Team
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ */
+
+#include "website/website_common.h"
+
+#include <SDL.h>
+
+#include "base/i18n.h"
+#include "base/wexception.h"
+#include "graphic/graphic.h"
+#include "io/filesystem/filesystem.h"
+#include "io/filesystem/layered_filesystem.h"
+#include "sound/sound_handler.h"
+
+// Setup the static objects Widelands needs to operate and initializes systems.
+void initialize() {
+	i18n::set_locale("en");
+
+	if (SDL_Init(SDL_INIT_VIDEO) != 0) {
+		throw wexception("Unable to initialize SDL: %s", SDL_GetError());
+	}
+
+	g_fs = new LayeredFileSystem();
+	g_fs->add_file_system(&FileSystem::create(INSTALL_DATADIR));
+
+	// We don't really need graphics or sound here, but we will get error messages
+	// when they aren't initialized
+	g_gr = new Graphic();
+	g_gr->initialize(Graphic::TraceGl::kNo, 1, 1, false);
+
+	g_sound_handler.init();
+	g_sound_handler.nosound_ = true;
+}
+
+// Cleanup before program end
+void cleanup() {
+	g_sound_handler.shutdown();
+
+	if (g_gr) {
+		delete g_gr;
+		g_gr = nullptr;
+	}
+
+	if (g_fs) {
+		delete g_fs;
+		g_fs = nullptr;
+	}
+
+	SDL_Quit();
+}

=== added file 'src/website/website_common.h'
--- src/website/website_common.h	1970-01-01 00:00:00 +0000
+++ src/website/website_common.h	2018-08-15 03:33:41 +0000
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2018 by the Widelands Development Team
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ */
+
+#ifndef WL_WEBSITE_WEBSITE_COMMON_H
+#define WL_WEBSITE_WEBSITE_COMMON_H
+
+// Setup the static objects Widelands needs to operate and initializes systems.
+void initialize();
+
+// Cleanup before program end
+void cleanup();
+
+#endif  // end of include guard: WL_WEBSITE_WEBSITE_COMMON_H