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

MaxPane can crash SublimeText #1785

Closed
deathaxe opened this issue Jun 25, 2017 · 8 comments
Closed

MaxPane can crash SublimeText #1785

deathaxe opened this issue Jun 25, 2017 · 8 comments

Comments

@deathaxe
Copy link
Collaborator

Summary

I often use MaxPane package to temporarily maximize a view in a multi group layout. Stupid but sometimes I forget a view is maximized and try to push a file to a second pane. This is the time Sublime Text reminds me about the maximized pane by crashing. It suddenly exists.

The issue was reported in 2014 at jisaacks/MaxPane#10

Expected behavior

Sublime Text may display a error message if something prevents it from creating a new pane and abort the new_pane command, but it should not crash.

Actual behavior

Sublime Text crashes and exits.

Steps to reproduce

  1. Create a 2 column layout
  2. Open a file in each group
  3. Focus the left view
  4. Run max_pane command to maximize the first group
  5. open a second file in the maximized pane
  6. Press ctrl+k, ctrl+up to move the file to second group (by new_pane)
  7. Crash!

or

  1. Create a 2 column layout
  2. Open a file in each group
  3. Focus the left view
  4. Run max_pane command to maximize the first group
  5. Press Shift+Alt+2 to create a new 2 column layout
  6. Crash!

Environment

  • Operating system and version:
    • Windows 10 x64
  • Monitor:
    • Resolution 1900x1200
    • dpi_scale used in ST 1.0
  • Sublime Text:
    • Build 3139
    • 64 bit
@keith-hall
Copy link
Collaborator

are you able to provide an example that doesn't require MaxPane to be installed, just to make it clearer what is happening/where it is crashing?

i.e. a specific combination of window.set_layout followed by new_pane? I guess changing that line to window.run_command('set_layout', l) makes no difference?

@deathaxe
Copy link
Collaborator Author

deathaxe commented Jun 28, 2017

I have a layout with two groups which window.get_layout() returns
{'cols': [0.0, 0.5, 1.0], 'rows': [0.0, 1.0], 'cells': [[0, 0, 1, 1], [1, 0, 2, 1]]} for.

By calling MaxPane while left pane is active, the layout is changed to:
{'cols': [0.0, 1.0, 1.0], 'rows': [0.0, 1.0], 'cells': [[0, 0, 1, 1], [1, 0, 2, 1]]}

I guess this means it just sets the maximized column/row to maximum relative width/height.

The maximized left group contains 3 views/documents.

Pressing Ctrl+K, Ctrlk+Up immediately crashes Sublime Text.

Running window.run_command("new_pane") from console works the first time - view is moved, but the second call crashes Sublime Text.

Doing the same thing with right group maximized causes both keybinding and running "new_pane" from console to crash the very first time.

I guess the width/height of 0.0 of the "hidden" views causes some layout calculation to fail maybe with division by 0?

@wbond
Copy link
Member

wbond commented Jun 29, 2017

This is almost certainly caused by the same thing as #961

@evandrocoan
Copy link

evandrocoan commented Aug 5, 2019

are you able to provide an example that doesn't require MaxPane to be installed

@keith-hall, I managed to create a minimal example by exhaustively testing MaxPane code.

  1. Close a Sublime Text Vanilla Install
  2. Create the plugin:
    import sublime
    import sublime_plugin
    
    class MaxPaneEvents(sublime_plugin.EventListener):
        def on_activated(self, view):
            w = view.window()
            w.set_layout({'rows': [0.0, 1.0], 'cols': [0.0, 0.5, 1.0], 'cells': [[0, 0, 1, 1], [1, 0, 2, 1]]})
  3. Create the keybind:
    {
        "keys": ["alt+shift+7"],
        "command": "set_layout",
        "args":
        {"cells": [[0, 0, 1, 1], [1, 0, 2, 1], [2, 0, 3, 1]], "cols": [0.0, 1.0, 1.0, 1.0], "rows": [0.0, 1.0]},
    },
  4. Open Sublime Text
  5. Press Alt+Shit+7
  6. Sublime Text Crashes

It instantaneously creates a dump file with the following: ae9e693d-13ca-484a-ae72-9b150d059cfd.zip

Dump Summary
------------
Dump File:  ae9e693d-13ca-484a-ae72-9b150d059cfd.dmp : F:\SublimeText\Vanilla\ae9e693d-13ca-484a-ae72-9b150d059cfd.dmp
Last Write Time:    04-Aug-19 22:47:35
Process Name:   sublime_text.exe : F:\SublimeText\Vanilla\sublime_text.exe
Process Architecture:   x64
Exception Code: 0xC0000005
Exception Information:  The thread tried to read from or write to a virtual address for which it does not have the appropriate access.
Heap Information:   Not Present

System Information
------------------
OS Version: 10.0.15063
CLR Version(s):

Modules
-------
Module Name Module Path Module Version
----------- ----------- --------------
sublime_text.exe    F:\SublimeText\Vanilla\sublime_text.exe 1.0.0.1
ntdll.dll   C:\Windows\System32\ntdll.dll   10.0.15063.1155
kernel32.dll    C:\Windows\System32\kernel32.dll    10.0.15063.1058
...

Environment

  • Operating system and version:
    • Windows 10 build 15063 x64
    • Resolution 1920x1080
    • dpi_scale 1.0
    • Sublime Text:
    • Build 3207
    • 64 bit

@deathaxe, while searching for the cause of the crashed, I figured out you can fix the MaxPane package by changing these lines:

        w.set_layout(l)

            w.set_layout(PaneManager.popLayout(w))

        w.set_layout(l)

# -->
        sublime.set_timeout( lambda: w.set_layout(l), 100 )

            layout = PaneManager.popLayout(w)
            sublime.set_timeout( lambda: w.set_layout( layout ), 100 )

        sublime.set_timeout( lambda: w.set_layout(l), 100 )

@wbond, the crash itself happens because MaxPane is trying to change the layout by the on_activated() event, which is triggered right after we set our layout with Alt+Shift+Keys.

When the layout is maximized, i.e., with the state:

  1. {'cells': [[0, 0, 1, 1], [1, 0, 2, 1]], 'rows': [0.0, 1.0], 'cols': [0.0, 1.0, 1.0]}

And we call the Alt+Shift+3 key, then Sublime Text correctly sets the layout, but the on_activated() event from MaxPane is triggered immediately trying to change the layout back to the Unmaximized State of the pane.

Adding the delay fixes the crash problem because now Sublime Text is not trying to change the layout twice in row and inside the on_activated() event, as there is a delay added to MaxPane layout change commands.

I tried adding a delay of 0 instead of 100 milliseconds, but when I tested, if I try to randomly keep maximizing panes and changing layouts highly often/fast, Sublime Text still crashing some random times, not easily reproducible.

@deathaxe
Copy link
Collaborator Author

deathaxe commented Aug 5, 2019

Great investigation. Can reproduce the crash with your example. Very helpfull.

But can't find the lines to change in max_pane.py in the current upstream master though.

But nevertheless your minimal example stops crashing if we use on_activated_async. This enables ST to set the maximized layout first before it is reverted by the asynchronously queued event listener.

class MaxPaneEvents(sublime_plugin.EventListener):
    def on_activated_async(self, view):
        w = view.window()
        w.set_layout({'rows': [0.0, 1.0], 'cols': [0.0, 0.5, 1.0], 'cells': [[0, 0, 1, 1], [1, 0, 2, 1]]})

If we don't like the flickering caused by the asynchronous event listener and in order to keep compatibility with ST2, the following fix works as well

class MaxPaneEvents(sublime_plugin.EventListener):
    def on_activated(self, view):
        def worker():
            w = view.window()
            w.set_layout({'rows': [0.0, 1.0], 'cols': [0.0, 0.5, 1.0], 'cells': [[0, 0, 1, 1], [1, 0, 2, 1]]})
        sublime.set_timeout(worker)

Package Control and other plugins which used to work for ST2 use set_timeout very often to queue functions and sync with the core.

deathaxe pushed a commit to jisaacks/MaxPane that referenced this issue Aug 5, 2019
Sublime Text can crash on certain layout changes, especially if
another layout change is still on the fly.

 see:  sublimehq/sublime_text#1785

This commit queues/delays the execution of the code in the
`on_activated()` handler to ensure the ST core can finish a layout
change before calling the next one.

A synchronous `set_timeout()` is used for backward compatibility with
ST2 and because of no flickering when manipulating the layout. The
10ms delay is also added for backward compatibility reasons and is not
needed for recent ST3 builds.

The `view` is passed using the `partial()` function to ensure the
queued worker uses the correct and valid instance.

Note:

  The event handlers don't need to return a value like `None`.
deathaxe pushed a commit to jisaacks/MaxPane that referenced this issue Aug 6, 2019
Sublime Text can crash on certain layout changes, especially if
another layout change is still on the fly.

 see:  sublimehq/sublime_text#1785

This commit queues/delays the execution of the code in the
`on_activated()` handler to ensure the ST core can finish a layout
change before calling the next one.

A synchronous `set_timeout()` is used for backward compatibility with
ST2 and because of no flickering when manipulating the layout. The
10ms delay is also added for backward compatibility reasons and is not
needed for recent ST3 builds.

The `view` is passed using the `partial()` function to ensure the
queued worker uses the correct and valid instance.

Note:

  The event handlers don't need to return a value like `None`.

Fix partial doesn't work

The on_activated() is not called if partial() is used in the decorator.
Hence, remove it.

Fix on_window_command

Queue `on_window_command` the same way as `on_activate` causes strange
things to happen. Revert it.

Simplify decorator

The decorator's only job is to run a function delayed.
@deathaxe
Copy link
Collaborator Author

deathaxe commented Aug 7, 2019

Updated MaxPane for various reasons recently, including ...

Not sure whether it helps fixing occational crashes, but hope it makes things better.

@evandrocoan
Copy link

I did the same things as you, except I kept using run_command(), dropped support for Sublime Text 2 and I also edited my fork of the Origami package and integrated their "zoom" functionality/feature with the maximize pane feature. Now you can mix between them without any lost of layouts, i.e., create new panes, destroy panes, ...

  1. evandroforks/Origami@9d5556b
  2. evandrocoan/MaxPane@1136b91

@deathaxe
Copy link
Collaborator Author

Haven't seen crashs using MaxPane anymore.

Can't repreoduce this issue using the minimal examples in ST4 anymore.

@deathaxe deathaxe added this to the next dev cycle milestone Jul 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants