Memory leak ?

[nadrino]

I was trying to make my own overlay with this library, when I figured out Tesla made atmosphere crashing at some point.

In fact, I tried to reproduce this crash by providing an empty shell overlay module and loop with open/close. In addition I was able to reproduce this bug with any overlay module.

Do you have the same problem too ?

[HookedBehemoth]

https://github.com/nadrino/SimpleModManager/blob/master/ovl/source/main.cpp If this is the source code in question: You are leaking fs sessions.

[HookedBehemoth]

And you missunderstand how ScopeGuard works. The lambda will be executed as soon as you leave the scope.

So you'll unmount the sdcard filesystem as soon as you leave initServices

[nadrino]

Oh sorry I didn't update the repository while I was testing! Thanks a lot for clarrifying what the ScopeGuard was meant to do. In fact I stopped to use it because I couldn't lookup on the SD after the initialization.

Regarding the crash bug, it happend when I used a very minimal version of my program. In fact everything was commented and only the empty shell was running. Also as I said this bug is present for all the modules I have.

[HookedBehemoth]

So you now close the fs session leak and it still happens?

Could you share this minimal version or point to which other overlays you tested and experience the issue.

Also a crash report would be helpful. /atmosphere/crash_reports

[HookedBehemoth]

Checked on your repo. You still leak fs sessions. Remove fsInitialize.

[nadrino]

Oh yeah I forgot to remove it in the commit!

Ok, so in fact in seems that it is my mod_browser that is leaking... I don't know why it affected other tweaks tho.

Here you have a piece of my "minimal code":

#define TESLA_INIT_IMPL // If you have more than one file using the tesla header, only define this in the main one
#include <tesla.hpp>    // The Tesla Header
#include <toolbox.h>
#include <mod_browser.h>
#include <sstream>

mod_browser __mod_browser__;

class GameBrowserGui : public tsl::Gui {
public:

  // Called when this Gui gets loaded to create the UI
  // Allocate all elements on the heap. libtesla will make sure to clean them up when not needed anymore
  tsl::elm::Element* createUI() override {
    // A OverlayFrame is the base element every overlay consists of. This will draw the default Title and Subtitle.
    // If you need more information in the header or want to change it's look, use a HeaderOverlayFrame.
    _frame_ = new tsl::elm::OverlayFrame("SimpleModManager", "");

    // A list that can contain sub elements and handles scrolling

    // Return the frame to have it become the top level element of this Gui
    return _frame_;
  }

  // Called once every frame to update values
  void update() override {

  }

  // Called once every frame to handle inputs not handled by other UI elements
  bool handleInput(u64 keysDown, u64 keysHeld, touchPosition touchInput,
    JoystickPosition leftJoyStick, JoystickPosition rightJoyStick) override {

    return false;   // Return true here to singal the inputs have been consumed
  }

private:

  tsl::elm::OverlayFrame* _frame_;

};

class SimpleModManagerOverlay : public tsl::Overlay {
public:

  // libtesla already initialized fs, hid, pl, pmdmnt, hid:sys and set:sys
  void initServices() override {

    fsdevMountSdmc();

    tsl::hlp::ScopeGuard dirGuard([&] {
    });

  }  // Called at the start to initialize all services necessary for this Overlay
  void exitServices() override {

    fsdevUnmountDevice("sdmc");

  }  // Callet at the end to clean up all services previously initialized

  void onShow() override {}    // Called before overlay wants to change from invisible to visible state
  void onHide() override {}    // Called before overlay wants to change from visible to invisible state

  std::unique_ptr<tsl::Gui> loadInitialGui() override {
    return initially<GameBrowserGui>();  // Initial Gui to load. It's possible to pass arguments to it's constructor like this
  }

};

int main(int argc, char **argv) {
  return tsl::loop<SimpleModManagerOverlay>(argc, argv);
}

Here is the crash repport : report_0000000178d4beb8.bin.zip

Can you tell me how you look into these ? :)

Thanks for the support !

[nadrino]

I was able to make Atmosphere crash again with this even more simple piece of code. However I don't have any crash repport in the folder this time.

#define TESLA_INIT_IMPL // If you have more than one file using the tesla header, only define this in the main one
#include <tesla.hpp>    // The Tesla Header
#include <sstream>

class GameBrowserGui : public tsl::Gui {
public:

  // Called when this Gui gets loaded to create the UI
  // Allocate all elements on the heap. libtesla will make sure to clean them up when not needed anymore
  tsl::elm::Element* createUI() override {
    // A OverlayFrame is the base element every overlay consists of. This will draw the default Title and Subtitle.
    // If you need more information in the header or want to change it's look, use a HeaderOverlayFrame.
    _frame_ = new tsl::elm::OverlayFrame("SimpleModManager", "");

    // A list that can contain sub elements and handles scrolling

    // Return the frame to have it become the top level element of this Gui
    return _frame_;
  }

  // Called once every frame to update values
  void update() override {

  }

  // Called once every frame to handle inputs not handled by other UI elements
  bool handleInput(u64 keysDown, u64 keysHeld, touchPosition touchInput,
    JoystickPosition leftJoyStick, JoystickPosition rightJoyStick) override {

    return false;   // Return true here to singal the inputs have been consumed
  }

private:

  tsl::elm::OverlayFrame* _frame_;

};

class SimpleModManagerOverlay : public tsl::Overlay {
public:

  // libtesla already initialized fs, hid, pl, pmdmnt, hid:sys and set:sys
  void initServices() override {

    fsdevMountSdmc();

    tsl::hlp::ScopeGuard dirGuard([&] {
    });

  }  // Called at the start to initialize all services necessary for this Overlay
  void exitServices() override {

    fsdevUnmountDevice("sdmc");

  }  // Callet at the end to clean up all services previously initialized

  void onShow() override {}    // Called before overlay wants to change from invisible to visible state
  void onHide() override {}    // Called before overlay wants to change from visible to invisible state

  std::unique_ptr<tsl::Gui> loadInitialGui() override {
    return initially<GameBrowserGui>();  // Initial Gui to load. It's possible to pass arguments to it's constructor like this
  }

};

int main(int argc, char **argv) {
  return tsl::loop<SimpleModManagerOverlay>(argc, argv);
}

[masagrator]

Why you want to mount sdmc for all costs, when it's already mounted? You don't need to initialize sdcard in any NRO for hbmenu, so you don't need to do it in overlay.

In status monitor I'm not using any fs function at all, but opening file from sdcard still works.

[nadrino]

Oh I see... In fact I added this call because the "tsl::hlp::ScopeGuard". I didn't undertand why I could not reach any file so I added extra call to reopen sdmc

[nadrino]

Here is the crash log I had (after a fresh restart) with this version of the minimal code:

#define TESLA_INIT_IMPL // If you have more than one file using the tesla header, only define this in the main one
#include <tesla.hpp>    // The Tesla Header
#include <sstream>

class GameBrowserGui : public tsl::Gui {
public:

  GameBrowserGui(){
    _frame_=nullptr;
  }

  // Called when this Gui gets loaded to create the UI
  // Allocate all elements on the heap. libtesla will make sure to clean them up when not needed anymore
  tsl::elm::Element* createUI() override {
    // A OverlayFrame is the base element every overlay consists of. This will draw the default Title and Subtitle.
    // If you need more information in the header or want to change it's look, use a HeaderOverlayFrame.
    _frame_ = new tsl::elm::OverlayFrame("SimpleModManager", "");

    // A list that can contain sub elements and handles scrolling

    // Return the frame to have it become the top level element of this Gui
    return _frame_;
  }

  // Called once every frame to update values
  void update() override {

  }

  // Called once every frame to handle inputs not handled by other UI elements
  bool handleInput(u64 keysDown, u64 keysHeld, touchPosition touchInput,
    JoystickPosition leftJoyStick, JoystickPosition rightJoyStick) override {

    return false;   // Return true here to singal the inputs have been consumed
  }

private:

  tsl::elm::OverlayFrame* _frame_;

};

class SimpleModManagerOverlay : public tsl::Overlay {
public:

  // libtesla already initialized fs, hid, pl, pmdmnt, hid:sys and set:sys
  void initServices() override {

    tsl::hlp::ScopeGuard dirGuard([&] {
    });

  }  // Called at the start to initialize all services necessary for this Overlay
  void exitServices() override {

  }  // Callet at the end to clean up all services previously initialized

  void onShow() override {}    // Called before overlay wants to change from invisible to visible state
  void onHide() override {}    // Called before overlay wants to change from visible to invisible state

  std::unique_ptr<tsl::Gui> loadInitialGui() override {
    return initially<GameBrowserGui>();  // Initial Gui to load. It's possible to pass arguments to it's constructor like this
  }

};

int main(int argc, char **argv) {
  return tsl::loop<SimpleModManagerOverlay>(argc, argv);
}

01591114629_420000000007e51a.log

[HookedBehemoth]

Having a hard time reproducing it.

Realized that the vsync event never gets closed. Could you try adding the following line into tsl::gfx::Renderer::exit() eventClose(&this->m_vsyncEvent);

Compile and run both your overlay and tesla menu with that fix.

[nadrino]

Alright, should I add it before "framebufferClose(&this->m_framebuffer);" or after "viExit();"?

[nadrino]

This is usually a lot harder to reproduce with an empty shell.

And I don't know if it's this fix or not, but it was a lot harder than before to make it break.

Here are the two crash repports: (there is actually 2 repports when this happend) 01591127213_420000000007e51a.log 01591127212_420000000007e51a.log

[nadrino]

Here is a capture of the crash. This video might seem stupid because no user would do that forcing in any case, but when you have heavier ovl loaded its crashed more often.

[HookedBehemoth]

You still use release Tesla Menu

[HookedBehemoth]

Recompile that with the same "fix"

[nadrino]

Hmm I recompiled it, but the version in the repo says 1.1.1 (Makefile)

Do I just have to build "ovlmenu.ovl" and put it into the "/switch/.overlays/" folder ?

[nadrino]

Oh the version which is displayed is the ovlloader! I will try to recompile it

[HookedBehemoth]

Forgot that ovlmenu/tesla menu shows the version of the ovlloader and not it's own version.

Make sure to recompile it. To elaborate: The bug is in libtesla so every overlay compiled with the old version causes issues. Only start overlays in testing that have the fix applied.

[nadrino]

Ok so with only SMM and the recompiled ovl I was not able to reproduce this bug so far

[nadrino]

Here is a video of me trying to make it crash:

The "_fix" after SMM version is actually part of ovlmenu's code

[masagrator]

Oh I see... In fact I added this call because the "tsl::hlp::ScopeGuard". I didn't undertand why I could not reach any file so I added extra call to reopen sdmc

Sorry about that. I'm now making new overlay and it looks like also can't access sdcard files without manually mounting sdmc.

[nadrino]

I think it's a good thing that we have to mount sdmc in our own module. Such a call take few milliseconds in the start/stop process, and some ovl modules won't need this at all.