uosc icon indicating copy to clipboard operation
uosc copied to clipboard

Repeatedly scrolling through the menu and then clicking on it will crashes

Open dyphire opened this issue 1 year ago • 7 comments

Trying to open the uosc menu and clicking on it after scrolling repeatedly triggers the script to crash.

https://github.com/tomasklaen/uosc/assets/61936050/406871fd-7c39-47aa-8d59-52a1eaf30e5e

The error log contents:

[  77.009][w][uosc] 
[  77.009][w][uosc] stack traceback:
[  77.009][w][uosc] 	.../mpv-test/portable_config/scripts/uosc/elements/Menu.lua:665: in function 'handle_wheel_up'
[  77.009][w][uosc] 	.../mpv-test/portable_config/scripts/uosc/elements/Menu.lua:1119: in function 'handler'
[  77.009][w][uosc] 	...yer/mpv-test/portable_config/scripts/uosc/lib/cursor.lua:157: in function 'trigger'
[  77.009][w][uosc] 	...yer/mpv-test/portable_config/scripts/uosc/lib/cursor.lua:373: in function 'cb'
[  77.009][w][uosc] 	mp.defaults:107: in function 'fn'
[  77.009][w][uosc] 	mp.defaults:66: in function 'handler'
[  77.009][w][uosc] 	mp.defaults:383: in function 'handler'
[  77.009][w][uosc] 	mp.defaults:513: in function 'call_event_handlers'
[  77.009][w][uosc] 	mp.defaults:555: in function 'dispatch_events'
[  77.009][w][uosc] 	mp.defaults:506: in function <mp.defaults:505>
[  77.009][w][uosc] 	[C]: at 0x7ff7d9bce040
[  77.009][w][uosc] 	[C]: at 0x7ff7d9bcd670
[  77.009][f][uosc] Lua error: .../mpv-test/portable_config/scripts/uosc/elements/Menu.lua:425: attempt to index local 'menu' (a nil value)
[  77.013][d][uosc] Exiting...

mpv.log

dyphire avatar Jun 05 '24 15:06 dyphire

I can't reproduce that, and I can't see how that even can happen just from thinking through the code.

Menu.current needs to be nil for that crash to happen, but the only place where it's set to nil is in close(), which happens when the menu gets replaced with a new one for the path selected. However it also gets immediately replaced with a new one, and it doesn't fit the stack trace that it somehow happens in between that.

According to the stack trace you get a wheel up event when clicking on the .. which makes no sense at all :confused:

Right before the crash the log contains Set property: user-data/uosc/menu/type="open-file" -> 1, which happens right before the new menu gets filled with entries and Menu.current gets set. I guess you could insert some print/log statement after the self:update(data) in Menu:init() to make absolutely sure it doesn't crash somewhere in there, but that wouldn't make sense based on the stack trace...

Does that only happen when you go back out to the drives menu? Because that's windows specific (although I still don't see how that could happen there, but that would explain why I can't reproduce it).

christoph-heinrich avatar Jun 05 '24 18:06 christoph-heinrich

Does that only happen when you go back out to the drives menu? Because that's windows specific (although I still don't see how that could happen there, but that would explain why I can't reproduce it).

After several tests it can be confirmed that the crash only occurs when trying to return to the drive menu and works fine in other subdirectories.

dyphire avatar Jun 06 '24 14:06 dyphire

Thanks for finding that out.

@tomasklaen can you reproduce that?

christoph-heinrich avatar Jun 07 '24 11:06 christoph-heinrich

Nope :( Can't get it to crash by scrolling and clicking up to drive menu.

tomasklaen avatar Jun 07 '24 15:06 tomasklaen

Unfortunately I can reproduce this crash consistently on my side. By the way after testing again, I reproduced the crash both when returning to the parent directory menu and when going to the subdirectory menu, so it's not actually only when returning to the drive menu.

Here's the new crash log file: mpv1.log , mpv2.log

Edit: Not sure if this is related to luajit.

dyphire avatar Jun 07 '24 15:06 dyphire

I'm guessing it's probably some sort of problem created by mouse events, and a simple patch would fix the crash for me:

@@ -422,7 +422,9 @@ end
 ---@param fling_options? Fling
 function Menu:scroll_by(delta, menu, fling_options)
 	menu = menu or self.current
-	self:scroll_to((menu.fling and (menu.fling.y + menu.fling.distance) or menu.scroll_y) + delta, menu, fling_options)
+	if menu ~= nil then
+		self:scroll_to((menu.fling and (menu.fling.y + menu.fling.distance) or menu.scroll_y) + delta, menu, fling_options)
+	end
 end
 
 ---@param index? integer
-- 

dyphire avatar Jun 07 '24 17:06 dyphire

That prevents the crash, but that should never even get called when self.current == nil, so there is a deeper problem somewhere.

christoph-heinrich avatar Jun 07 '24 18:06 christoph-heinrich

Fixed by https://github.com/tomasklaen/uosc/commit/7f054a45f80755f88116ed5f3ae3650c94233b3d

dyphire avatar Aug 29 '24 00:08 dyphire