LadyDefile / Wordsmith-DalamudPlugin

Wordsmith is a Dalamud Plugin aiming to make roleplay easier, more convenient, and harder to accidentally "wrong chat" your post.
7 stars 3 forks source link

Closing Scratch Pad from Settings crashes game #15

Closed NPittinger closed 2 years ago

NPittinger commented 2 years ago

Steps to reproduce:

  1. Select Tell in Scratch Pad from any online friend on the Friend List.
  2. Open Wordsmith settings, navigate to Scratch Pad tab.
  3. Click the Close button associated with the open scratch pad.

Traceback:

2022-07-24 09:25:08.030 -04:00 [ERR] [Wordsmith] System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
   at ImGuiNET.ImGuiNative.igTableNextColumn()
   at Wordsmith.Gui.SettingsUI.DrawScratchPadTab() in C:\Users\EilaD\source\repos\Wordsmith\Wordsmith\Gui\SettingsUI.cs:line 312
   at Wordsmith.Gui.SettingsUI.Draw() in C:\Users\EilaD\source\repos\Wordsmith\Wordsmith\Gui\SettingsUI.cs:line 141
   at Dalamud.Interface.Windowing.Window.DrawInternal() in C:\goatsoft\companysecrets\dalamud\Interface\Windowing\Window.cs:line 200
   at Dalamud.Interface.Windowing.WindowSystem.Draw() in C:\goatsoft\companysecrets\dalamud\Interface\Windowing\WindowSystem.cs:line 117
   at Wordsmith.WordsmithUI.Draw() in C:\Users\EilaD\source\repos\Wordsmith\Wordsmith\WordsmithUI.cs:line 104 :: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
LadyDefile commented 2 years ago

Yep. Seems to reliably crash the game. I'll spend some time combing through it and seeing if I can't figure out why.

LadyDefile commented 2 years ago

I believe I have worked out why the issue happens and have already built a fix for the issue. It needs more testing before I roll it out but I believe the fix should resolve the bug permanently and should also remove the possibility of similar, as-of-yet-undetected bugs. The technical side of things, if you're curious:

The issue:

When closing a Window from another Window, it was possible to cause ImGui to fail to draw because it would attempt to draw the disposed Window.

The fix:

I implemented a removal queue system that does window removal between draw cycles.

During the draw cycle of Wordsmith, the WindowSystem becomes locked. If, for any reason, a call to remove a Window happens during the draw cycle, the removal is deferred. At the end of the draw cycle, the WindowSystem is unlocked. After unlocking the system, the deferred windows are then removed.

This should effectively eliminate the possibility of Windows getting corrupted from deletion.