Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash accessing theme dimensions/info in a plugin's init() function #3913

Closed
sentry-io bot opened this issue Jun 5, 2023 · 5 comments
Closed

Crash accessing theme dimensions/info in a plugin's init() function #3913

sentry-io bot opened this issue Jun 5, 2023 · 5 comments
Assignees
Labels
bug crash report Issue originated from a crash report / .dmp file
Milestone

Comments

@sentry-io
Copy link

sentry-io bot commented Jun 5, 2023

Sentry Issue: ASEPRITE-14Q

OS Version: Windows 10.0.19045 (2965)
Report Version: 104

Crashed Thread: 2808

Application Specific Information:
Fatal Error: EXCEPTION_ACCESS_VIOLATION_READ / 0x1ad0

Thread 2808 Crashed:
0   aseprite.exe                    0x7ff7400a44e1      [inlined] std::_Tree<T>::_Find_lower_bound (xtree:1611)
1   aseprite.exe                    0x7ff7400a44e1      [inlined] std::_Tree<T>::_Find (xtree:1367)
2   aseprite.exe                    0x7ff7400a44e1      [inlined] std::_Tree<T>::find (xtree:1381)
3   aseprite.exe                    0x7ff7400a44e1      [inlined] app::skin::SkinTheme::getDimensionById (skin_theme.h:139)
4   aseprite.exe                    0x7ff7400a44e1      [inlined] app::script::`anonymous-namespace'::Theme::getDimensionById (app_theme_object.cpp:41)
5   aseprite.exe                    0x7ff7400a44e1      [inlined] app::script::`anonymous-namespace'::ThemeDimension::getById (app_theme_object.cpp:57)
6   aseprite.exe                    0x7ff7400a44e1      app::script::`anonymous namespace'::ThemeDimension_index (app_theme_object.cpp:88)
7   aseprite.exe                    0x7ff7402d8b7d      luaD_precall (ldo.c:532)
8   aseprite.exe                    0x7ff7402d826b      ccall (ldo.c:575)
9   aseprite.exe                    0x7ff7402e2774      luaT_callTMres (ltm.c:129)
10  aseprite.exe                    0x7ff7402e1e69      luaV_finishget (lvm.c:309)
11  aseprite.exe                    0x7ff7402dfa68      luaV_execute (lvm.c:1158)
12  aseprite.exe                    0x7ff7402d8284      ccall (ldo.c:577)
13  aseprite.exe                    0x7ff7402d8f12      luaD_rawrunprotected (ldo.c:144)
14  aseprite.exe                    0x7ff7402d894f      luaD_pcall (ldo.c:892)
15  aseprite.exe                    0x7ff7402d3f4d      lua_pcallk (lapi.c:1057)
16  aseprite.exe                    0x7ff73fe2c59e      app::script::Engine::evalCode (engine.cpp:537)
17  aseprite.exe                    0x7ff73fe2cc71      app::script::Engine::evalFile (engine.cpp:593)
18  aseprite.exe                    0x7ff73fe2cec5      app::script::Engine::evalUserFile (engine.cpp:610)
19  aseprite.exe                    0x7ff73ffa5249      app::RunScriptCommand::onExecute (cmd_run_script.cpp:79)
20  aseprite.exe                    0x7ff73fd8480c      app::Context::executeCommand (context.cpp:188)
21  aseprite.exe                    0x7ff740072306      app::script::`anonymous namespace'::Command_call (app_command_object.cpp:54)
22  aseprite.exe                    0x7ff7402d8b7d      luaD_precall (ldo.c:532)
23  aseprite.exe                    0x7ff7402e145a      luaV_execute (lvm.c:1624)
24  aseprite.exe                    0x7ff7402d8284      ccall (ldo.c:577)
25  aseprite.exe                    0x7ff7402d8f12      luaD_rawrunprotected (ldo.c:144)
26  aseprite.exe                    0x7ff7402d894f      luaD_pcall (ldo.c:892)
27  aseprite.exe                    0x7ff7402d3f4d      lua_pcallk (lapi.c:1057)
28  aseprite.exe                    0x7ff73fdaa8e7      app::Extension::initScripts (extensions.cpp:677)
29  aseprite.exe                    0x7ff73fda92a7      app::Extensions::executeInitActions (extensions.cpp:853)
30  aseprite.exe                    0x7ff73fd0998a      app::App::initialize (app.cpp:401)
31  aseprite.exe                    0x7ff73fd0238a      app_main (main.cpp:143)
32  aseprite.exe                    0x7ff74049e635      wWinMain (main.cpp:40)
33  aseprite.exe                    0x7ff740412b29      [inlined] invoke_main (exe_common.inl:118)
34  aseprite.exe                    0x7ff740412b29      __scrt_common_main_seh (exe_common.inl:288)
35  KERNEL32.DLL                    0x7ffb9cee7613      BaseThreadInitThunk
36  ntdll.dll                       0x7ffb9d0226a0      RtlUserThreadStart
@dacap dacap added bug crash report Issue originated from a crash report / .dmp file labels Jun 5, 2023
@dacap dacap added this to the v1.3-rc5 milestone Jun 5, 2023
@Gasparoken
Copy link
Member

I couldn't reproduce it yet.

@dacap
Copy link
Member

dacap commented Jun 6, 2023

From the stacktrace it looks like a plugin ( https://www.aseprite.org/api/plugin ) accessed a dimension from app.theme.dimension from its init() function, anyway I was able to reproduce a similar crash with this script:

local dim = app.theme.dimension
local dialog = Dialog("Test")
dialog:button{ text="Click", onclick=function()
 app.alert(tostring(dim.scrollbar_size))
end}
dialog:show{ wait=false }

The issue is that ThemeDimension keeps a reference to a freed theme (probably we shouldn't store the Theme* pointer, and just use the SkinTheme::instance()).

@Gasparoken
Copy link
Member

Even with the script, I couldn't reproduce the crash (Windows and macOS). Anyway your consideration is a nice improvement.

As you indicate, the structure of ThemeDimension can be modified from this:

struct ThemeDimension {
  Theme* theme;

  ThemeDimension(Theme* theme) : theme(theme) { }

  int getById(const std::string& id) const {
    return theme->getDimensionById(id);
  }
};

To a pointer-less version without side effects (not fully tested yet):

struct ThemeDimension {
  int getById(const std::string& id) const {
    return skin::SkinTheme::instance()->getDimensionById(id);
  }
};

@dacap
Copy link
Member

dacap commented Jun 7, 2023

Even with the script, I couldn't reproduce the crash (Windows and macOS). Anyway your consideration is a nice improvement.

See if opening the script, refreshing (F5) and then clicking the dialog button produces the crash. Compile with the memory sanitizer.

@Gasparoken
Copy link
Member

I created a test plugin and used app.theme.dimension in the plugin's init() function but still can't reproduce the crash (F5 didn't help, tested on macOS and Windows)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment