Fixing Simula's memory leaks

[georgewsinger]

Launching multiple apps in Simula causes an ascending memory/CPU drain, until Simula eventually freezes. Here are the apps that I'm launching:

launchSimulaApps() {
    pkill firefox
    pkill xfce4-terminal
    pkill libreoffice
    pkill evince
    pkill emacs
    pkill gvim

    DISPLAY=:2 xfce4-terminal &
    sleep 5;
    DISPLAY=:2 firefox https://www.youtube.com/watch?v=Ii3PDjJCCQ4 &
    sleep 5;
    DISPLAY=:2 xfce4-terminal --execute htop &
    sleep 5;
    DISPLAY=:2 evince /home/george/Downloads/SimulaPitchBook.pdf &
    sleep 5;
    DISPLAY=:2 emacs &
    sleep 5;
    DISPLAY=:2 evince /home/george/Downloads/AoS.pdf &
}

Using shell script

logpid() { while sleep 1; do  ps -p $1 -o pcpu= -o pmem= ; done; }

to log CPU + memory usage yields this data, which can be plotted as

[georgewsinger]

There are many candidates for potential leaks in gdwlroots:

george@UbuntuBox:~/Simula/build/godot/modules/gdwlroots$ grep -nr "return new"
wlr_xdg_shell.cpp:135:  return new WlrSurfaceAtResult(
wlr_xdg_shell.cpp:226:  return new WlrXdgSurface(xdg_surface);
wlr_xdg_shell.cpp:344:  return new WlrXdgToplevel(xdg_toplevel);
wlr_xdg_shell.cpp:528:  return new WlrXdgPopup(xdg_popup);
wlr_xwayland.cpp:120:  return new WlrSurfaceAtResult(WlrSurface::from_wlr_surface(result), sub_x, sub_y);
wlr_xwayland.cpp:175:  return new WlrXWaylandSurface(xwayland_surface);
wlr_surface.cpp:61: return new WlrSurfaceState(&wlr_surface->current);
wlr_surface.cpp:65: return new WlrSurfaceState(&wlr_surface->pending);
wlr_surface.cpp:69: return new WlrSurfaceState(&wlr_surface->previous);
wlr_surface.cpp:122:    return new WlrSurface(surface);

[georgewsinger]

Launching a single app in Simula shows traces of CPU/Memory leaks.

EDIT: Leaving Firefox on for a little more while shows CPU stabalizing:

EDIT2: Even more time:

[georgewsinger]

Let's work down from the grep cases above:

wlr_xdg_shell.cpp. We aren't using XDG apps in the examples above, so we can omit this for now.

wlr_xwayland.cpp.

//Leaks, but we clean up in Haskell (see below).
WlrSurfaceAtResult *WlrXWaylandSurface::surface_at(double sx, double sy) {
  double sub_x, sub_y;
  struct wlr_surface *result = wlr_surface_surface_at(wlr_xwayland_surface->surface, sx, sy, &sub_x, &sub_y);

  return new WlrSurfaceAtResult(WlrSurface::from_wlr_surface(result), sub_x, sub_y);
}

//Don't think this leaks much (or at all)
WlrXWaylandSurface *WlrXWaylandSurface::from_wlr_xwayland_surface(
    struct wlr_xwayland_surface *xwayland_surface) {
  if (xwayland_surface->data) {
    return (WlrXWaylandSurface *)xwayland_surface->data; //<- 90% sure this is most common path
  }
  return new WlrXWaylandSurface(xwayland_surface);
}

wlr_surface.cpp.

//Shouldn't leak much
WlrSurface *WlrSurface::from_wlr_surface(struct wlr_surface *surface) {
  if (!surface) {
    return NULL;
  }
  if (surface->data) {
    auto s = (WlrSurface *)surface->data; //<- Should be most common path, but am less sure than analogous case in wlr_xwayland_surface.cpp
    return s;
  }
  return new WlrSurface(surface);
}

//Leaks, but we clean up from Haskell
WlrSurfaceState *WlrSurface::alloc_current_state() const {
  return new WlrSurfaceState(&wlr_surface->current);
}

WlrSurfaceState *WlrSurface::alloc_pending_state() const {
  return new WlrSurfaceState(&wlr_surface->pending);
}

WlrSurfaceState *WlrSurface::alloc_previous_state() const {
  return new WlrSurfaceState(&wlr_surface->previous);
}

void WlrSurfaceAtResult::delete_surface_at_result() {
    delete this;
}

void WlrSurfaceState::delete_state() {
  delete this;
}

getBufferDimensions :: GodotWlrSurface -> IO (Int, Int)
getBufferDimensions wlrSurface = do
  dims@(bufferWidth, bufferHeight) <- withCurrentState $ getBufferDimensions'
  return dims
  where withCurrentState :: (GodotWlrSurfaceState -> IO b) -> IO b
        withCurrentState stateAction = do
          wlrSurfaceState <- G.alloc_current_state wlrSurface
          ret             <- stateAction wlrSurfaceState
          G.delete_state wlrSurfaceState -- <-- We clean up here
          return ret

getXWaylandSubsurfaceAndCoords :: GodotWlrXWaylandSurface -> SurfaceLocalCoordinates -> IO (GodotWlrSurface, SubSurfaceLocalCoordinates)
getXWaylandSubsurfaceAndCoords wlrXWaylandSurface (SurfaceLocalCoordinates (sx, sy)) = do
  ret@(wlrSurface', SubSurfaceLocalCoordinates (subX, subY)) <- withGodot
    (G.surface_at wlrXWaylandSurface sx sy)
    G.delete_surface_at_result -- <-- We clean up here
    (\wlrSurfaceAtResult -> do subX <- G.get_sub_x wlrSurfaceAtResult
                                subY <- G.get_sub_y wlrSurfaceAtResult
                                wlrSurface' <- G.get_surface wlrSurfaceAtResult
                                return (wlrSurface', SubSurfaceLocalCoordinates (subX, subY)))
  return ret

[georgewsinger]

godot-haskell has several memory leaks. For example, every time we toLowLevel from (Variant 'GodotTy) to GodotVariant:

instance GodotFFI GodotVariant (Variant 'GodotTy) where
  --…

  toLowLevel VariantNil = godot_variant_new_nil
  toLowLevel (VariantBool b) = godot_variant_new_bool . fromIntegral $ fromEnum b
  toLowLevel (VariantInt i) = godot_variant_new_int (fromIntegral i)
  toLowLevel (VariantReal r) = godot_variant_new_real (realToFrac r)
  toLowLevel (VariantString x) = godot_variant_new_string x
  toLowLevel (VariantVector2 x) = godot_variant_new_vector2 x
  toLowLevel (VariantRect2 x) = godot_variant_new_rect2 x
  toLowLevel (VariantVector3 x) = godot_variant_new_vector3 x
  toLowLevel (VariantTransform2d x) = godot_variant_new_transform2d x
  toLowLevel (VariantPlane x) = godot_variant_new_plane x
  toLowLevel (VariantQuat x) = godot_variant_new_quat x
  toLowLevel (VariantAabb x) = godot_variant_new_aabb x
  toLowLevel (VariantBasis x) = godot_variant_new_basis x
  toLowLevel (VariantTransform x) = godot_variant_new_transform x
  toLowLevel (VariantColor x) = godot_variant_new_color x
  toLowLevel (VariantNodePath x) = godot_variant_new_node_path x
  toLowLevel (VariantRid x) = godot_variant_new_rid x
  toLowLevel (VariantObject x) = godot_variant_new_object x
  toLowLevel (VariantDictionary x) = godot_variant_new_dictionary x
  toLowLevel (VariantArray x) = godot_variant_new_array x
  toLowLevel (VariantPoolByteArray x) = godot_variant_new_pool_byte_array x
  toLowLevel (VariantPoolIntArray x) = godot_variant_new_pool_int_array x
  toLowLevel (VariantPoolRealArray x) = godot_variant_new_pool_real_array x
  toLowLevel (VariantPoolStringArray x) = godot_variant_new_pool_string_array x
  toLowLevel (VariantPoolVector2Array x) = godot_variant_new_pool_vector2_array x
  toLowLevel (VariantPoolVector3Array x) = godot_variant_new_pool_vector3_array x
  toLowLevel (VariantPoolColorArray x) = godot_variant_new_pool_color_array x

Significant progress obtained after fixing fromVariant leaks in SimulaViewSprite.hs. Two of the most important leaks were found:

modified   addons/godot-haskell-plugin/src/Plugin/SimulaViewSprite.hs
@@ -416,6 +416,7 @@ processClickEvent gsvs evt clickPos = do
       case maybeWlrSurfaceGV of
           Nothing -> putStrLn "Failed to convert GodotWlrSurface to GodotVariant!"
           Just wlrSurfaceGV -> do G.keyboard_notify_enter wlrSeat wlrSurfaceGV
+                                  Api.godot_variant_destroy wlrSurfaceGV

     -- | This function conspiciously lacks a GodotWlrSurface argument, but doesn't
     -- | need one since the GodotWlrSeat keeps internal track of what the currently
@@ -452,6 +453,7 @@ pointerNotifyEnter wlrSeat wlrSurface (SubSurfaceLocalCoordinates (ssx, ssy)) =
   case maybeWlrSurfaceGV of
       Nothing -> putStrLn "Failed to convert GodotWlrSurface to GodotVariant!"
       Just wlrSurfaceGV -> do G.pointer_notify_enter wlrSeat wlrSurfaceGV ssx ssy -- Causing a crash
+                              Api.godot_variant_destroy wlrSurfaceGV
                               -- putStrLn $ "G.point_notify_enter: " ++ "(" ++ (show ssx) ++ ", " ++ (show ssy) ++ ")"

 _handle_map :: GodotSimulaViewSprite -> [GodotVariant] -> IO ()

[georgewsinger]

Launching Simula w/o any apps exhibits very slow leakage.

[georgewsinger]

Godotston exhibits no leakage.

[georgewsinger]

Here are the things we have to free manually in godot-haskell/Simula:

[georgewsinger]

Fixing NodePath leaks shows some progress?

---

EDIT: Yes -- some progress. Usually launching Simula w/several apps is unstable after 2 or so minutes. We can now use apps for several minutes before a crash.

[georgewsinger]

:heavy_check_mark: Pushed some progress here.

[georgewsinger]

Simula stress test: launching several apps (including YouTube video). @KaneTW has made substantial progress on fixing Simula/gdwlroot's memory management:

We still very gradual memory loss, but nowhere near as bad as before.