Fixed memory leak in DrawingContext

DrawingContext was using placement-new to create objects, but not
manually calling the destructors on them, thus leaking memory when the
object in question itself allocated memory, i.e. std::strings in
TextRequest.

diff --git a/src/video/drawing_context.cpp b/src/video/drawing_context.cpp
index 96d06ab..e4b9ebc 100644 (file)
--- a/src/video/drawing_context.cpp
+++ b/src/video/drawing_context.cpp
@@ -53,10 +53,27 @@ DrawingContext::DrawingContext(Renderer& renderer, Lightmap& lightmap) :
 
 DrawingContext::~DrawingContext()
 {
+  clear_drawing_requests(lightmap_requests);
+  clear_drawing_requests(drawing_requests);
+
   obstack_free(&obst, NULL);
 }
 
 void
+DrawingContext::clear_drawing_requests(DrawingRequests& requests)
+{
+  for(auto& request : requests)
+  {
+    if (request->request_data)
+    {
+      request->request_data->~DrawingRequestData();
+    }
+    request->~DrawingRequest();
+  }
+  requests.clear();
+}
+
+void
 DrawingContext::draw_surface(SurfacePtr surface, const Vector& position,
                              float angle, const Color& color, const Blend& blend,
                              int layer)
@@ -81,7 +98,9 @@ DrawingContext::draw_surface(SurfacePtr surface, const Vector& position,
   request->color = color;
   request->blend = blend;
 
-  request->request_data = surface.get();
+  SurfaceRequest* surfacerequest = new(obst) SurfaceRequest();
+  surfacerequest->surface = surface.get();
+  request->request_data = surfacerequest;
 
   requests->push_back(request);
 }
@@ -236,7 +255,7 @@ DrawingContext::draw_filled_rect(const Rectf& rect, const Color& color, float ra
   fillrectrequest->radius = radius;
   request->request_data = fillrectrequest;
 
-  requests->push_back(request); 
+  requests->push_back(request);
 }
 
 void
@@ -253,20 +272,20 @@ DrawingContext::draw_inverse_ellipse(const Vector& pos, const Vector& size, cons
   request->alpha = transform.alpha;
 
   InverseEllipseRequest* ellipse = new(obst)InverseEllipseRequest;
-  
+
   ellipse->color        = color;
   ellipse->color.alpha  = color.alpha * transform.alpha;
   ellipse->size         = size;
   request->request_data = ellipse;
 
-  requests->push_back(request);     
+  requests->push_back(request);
 }
 
 Rectf
 DrawingContext::get_cliprect() const
 {
   return Rectf(get_translation().x, get_translation().y,
-               get_translation().x + SCREEN_WIDTH, 
+               get_translation().x + SCREEN_WIDTH,
                get_translation().y + SCREEN_HEIGHT);
 }
 
@@ -322,10 +341,12 @@ DrawingContext::do_drawing()
     request->layer = LAYER_HUD - 1;
     drawing_requests.push_back(request);
   }
-  lightmap_requests.clear();
+
+  clear_drawing_requests(lightmap_requests);
 
   handle_drawing_requests(drawing_requests);
-  drawing_requests.clear();
+  clear_drawing_requests(drawing_requests);
+
   obstack_free(&obst, NULL);
   obstack_init(&obst);
 
@@ -495,7 +516,7 @@ DrawingContext::set_ambient_color( Color new_color )
   ambient_color = new_color;
 }
 
-void 
+void
 DrawingContext::take_screenshot()
 {
   screenshot_requested = true;

diff --git a/src/video/drawing_context.hpp b/src/video/drawing_context.hpp
index 13f28c9..6b782c2 100644 (file)
--- a/src/video/drawing_context.hpp
+++ b/src/video/drawing_context.hpp
@@ -141,7 +141,7 @@ private:
 
     Transform() :
       translation(),
-      drawing_effect(NO_EFFECT), 
+      drawing_effect(NO_EFFECT),
       alpha(1.0f)
     { }
 
@@ -151,6 +151,8 @@ private:
     }
   };
 
+  void clear_drawing_requests(DrawingRequests& requests);
+
 private:
   Renderer& renderer;
   Lightmap& lightmap;

diff --git a/src/video/drawing_request.hpp b/src/video/drawing_request.hpp
index 757d199..669c923 100644 (file)
--- a/src/video/drawing_request.hpp
+++ b/src/video/drawing_request.hpp
@@ -44,11 +44,11 @@ enum {
   LAYER_OBJECTS = 50,
   // Objects that pass through walls
   LAYER_FLOATINGOBJECTS = 150,
-  // 
+  //
   LAYER_FOREGROUNDTILES = 200,
-  // 
+  //
   LAYER_FOREGROUND0 = 300,
-  // 
+  //
   LAYER_FOREGROUND1 = 400,
   // Hitpoints, time, coins, etc.
   LAYER_HUD = 500,
@@ -80,7 +80,26 @@ enum RequestType
   SURFACE, SURFACE_PART, TEXT, GRADIENT, FILLRECT, INVERSEELLIPSE, DRAW_LIGHTMAP, GETLIGHT
 };
 
-struct SurfacePartRequest
+struct DrawingRequestData
+{
+  virtual ~DrawingRequestData()
+  {}
+};
+
+struct SurfaceRequest : public DrawingRequestData
+{
+  SurfaceRequest() :
+    surface()
+  {}
+
+  const Surface* surface;
+
+private:
+  SurfaceRequest(const SurfaceRequest&) = delete;
+  SurfaceRequest& operator=(const SurfaceRequest&) = delete;
+};
+
+struct SurfacePartRequest : public DrawingRequestData
 {
   SurfacePartRequest() :
     surface(),
@@ -91,9 +110,13 @@ struct SurfacePartRequest
   const Surface* surface;
   Vector source;
   Vector size;
+
+private:
+  SurfacePartRequest(const SurfacePartRequest&) = delete;
+  SurfacePartRequest& operator=(const SurfacePartRequest&) = delete;
 };
 
-struct TextRequest
+struct TextRequest : public DrawingRequestData
 {
   TextRequest() :
     font(),
@@ -110,7 +133,7 @@ private:
   TextRequest& operator=(const TextRequest&);
 };
 
-struct GradientRequest
+struct GradientRequest : public DrawingRequestData
 {
   GradientRequest()  :
     top(),
@@ -123,7 +146,7 @@ struct GradientRequest
   Vector size;
 };
 
-struct FillRectRequest
+struct FillRectRequest : public DrawingRequestData
 {
   FillRectRequest() :
     color(),
@@ -136,7 +159,7 @@ struct FillRectRequest
   float  radius;
 };
 
-struct InverseEllipseRequest
+struct InverseEllipseRequest : public DrawingRequestData
 {
   InverseEllipseRequest() :
     color(),
@@ -160,7 +183,7 @@ struct DrawingRequest
   float angle;
   Color color;
 
-  void* request_data;
+  DrawingRequestData* request_data;
 
   DrawingRequest() :
     target(),
@@ -181,9 +204,15 @@ struct DrawingRequest
   }
 };
 
-struct GetLightRequest
+struct GetLightRequest : public DrawingRequestData
 {
+  GetLightRequest() : color_ptr() {}
+
   Color* color_ptr;
+
+private:
+  GetLightRequest(const GetLightRequest&) = delete;
+  GetLightRequest& operator=(const GetLightRequest&) = delete;
 };
 
 #endif

diff --git a/src/video/gl/gl_renderer.cpp b/src/video/gl/gl_renderer.cpp
index 6dc1df3..82e9c1a 100644 (file)
--- a/src/video/gl/gl_renderer.cpp
+++ b/src/video/gl/gl_renderer.cpp
@@ -111,7 +111,7 @@ GLRenderer::~GLRenderer()
 void
 GLRenderer::draw_surface(const DrawingRequest& request)
 {
-  const Surface* surface = (const Surface*) request.request_data;
+  const Surface* surface = static_cast<const SurfaceRequest*>(request.request_data)->surface;
   if(surface == NULL)
   {
     return;

diff --git a/src/video/sdl/sdl_painter.cpp b/src/video/sdl/sdl_painter.cpp
index 6ba8f42..8074d2f 100644 (file)
--- a/src/video/sdl/sdl_painter.cpp
+++ b/src/video/sdl/sdl_painter.cpp
@@ -58,7 +58,7 @@ SDL_BlendMode blend2sdl(const Blend& blend)
 void
 SDLPainter::draw_surface(SDL_Renderer* renderer, const DrawingRequest& request)
 {
-  const Surface* surface = (const Surface*) request.request_data;
+  const Surface* surface = static_cast<const SurfaceRequest*>(request.request_data)->surface;
   boost::shared_ptr<SDLTexture> sdltexture = boost::dynamic_pointer_cast<SDLTexture>(surface->get_texture());
 
   SDL_Rect dst_rect;