airsdk / Adobe-Runtime-Support

Report, track and discuss issues in Adobe AIR. Monitored by Adobe - and HARMAN - and maintained by the AIR community.
206 stars 11 forks source link

[Windows] Displaying `ContextMenu`/`NativeMenu` with many items cause crash #3563

Open itlancer opened 5 days ago

itlancer commented 5 days ago

Problem Description

Displaying ContextMenu/NativeMenu with many items (1069 or more) cause crash for Windows. For example, if you try to list fonts names installed in OS (if them a lot). Yes, it's not good practice to use context menus with a lot of elements. But at least it shouldn't cause application crash.

Tested with multiple AIR versions, even with latest AIR 51.1.2.2 with different Windows devices, different OS versions, different AIR applications with different architectures (32/64-bit). Same issue in all cases. With less items count (1068 or less) all works fine. There is no such issue with macOS and Linux.

Related issues: https://github.com/airsdk/Adobe-Runtime-Support/issues/193 https://github.com/airsdk/Adobe-Runtime-Support/issues/192 https://github.com/airsdk/Adobe-Runtime-Support/issues/191

Steps to Reproduce

Launch code below with any Windows device and click anywhere on stage. Application will try to show context menu with 1069 elements. Application example with sources attached. windows_contextmenu_many_items_crash.zip

package {
    import flash.display.Sprite;
    import flash.events.Event;
    import flash.events.MouseEvent;
    import flash.ui.ContextMenu;
    import flash.ui.ContextMenuItem;

    public class WindowsContextMenuManyItemsCrash extends Sprite {

        public function WindowsContextMenuManyItemsCrash() {
            addEventListener(Event.ADDED_TO_STAGE, addedToStage);
        }

        private function addedToStage(e:Event):void {
            removeEventListener(Event.ADDED_TO_STAGE, addedToStage);

            stage.addEventListener(MouseEvent.CLICK, click);
        }

        private function click(e:MouseEvent):void {
            var menu:ContextMenu = new ContextMenu();
            for (var i:uint = 0; i < 1169; i++){
                var item:ContextMenuItem = new ContextMenuItem("Item " + i);
                menu.customItems.push(item);
            }

            menu.display(stage, e.stageX, e.stageY);//This line cause crash
        }
    }
}

Actual Result: Application crash:

Faulting application name: windows_contextmenu_many_items_crash.exe, version: 1.0.0.0, time stamp: 0x672b6e2f
Faulting module name: Adobe AIR.dll, version: 51.1.2.2, time stamp: 0x672b700c
Exception code: 0xc0000005
Fault offset: 0x00000000004fb915
Faulting process id: 0x2068
Faulting application start time: 0x01db32d50e6e8a5f
Faulting application path: C:\windows_contextmenu_many_items_crash.app\windows_contextmenu_many_items_crash.exe
Faulting module path: C:\windows_contextmenu_many_items_crash.app\Adobe AIR\Versions\1.0\Adobe AIR.dll
Report Id: 6086d2a8-8d06-44d5-9de6-b692560c28f4
Faulting package full name: 
Faulting package-relative application ID: 

Expected Result: Context menu will be shown. Or AS3 exception should be thrown if it not possible (OS limitation).

Known Workarounds

Do not display context menus with 1169 or more elements. Write your own implementation for that.

ajwfrost commented 3 days ago

Yes that's an OS error that we've encountered and then not handled properly... can't do much to add further menu items, but we can adjust it so it doesn't crash; the NativeMenu implementation already has some error handling where it's meant to return null on failure i.e. in https://airsdk.dev/reference/actionscript/3.0/flash/display/NativeMenu.html, the addItem and addItemAt methods would return null if they failed, or else they return the item that was passed in.

Your example uses the ContetMenu class which wraps up some of the NativeMenu functionality and processes the array of custom items; there's no error handling in there, it just calls addItemAt in a loop, but it should now just continue without the menu items that couldn't be added. I can't see any way that you can check for the error condition for this case though.

thanks

itlancer commented 3 days ago

@ajwfrost but I can see the same crash using NativeMenu and addItem. And addItem for "last element" (which cause crash) doesn't return null. And try-catch also doesn't help. Maybe I do something wrong...

Here is sample:

package {
    import flash.display.Sprite;
    import flash.display.NativeMenu;
    import flash.display.NativeMenuItem;
    import flash.events.Event;
    import flash.events.MouseEvent;

    public class WindowsContextMenuManyItemsCrash extends Sprite {

        public function WindowsContextMenuManyItemsCrash() {
            addEventListener(Event.ADDED_TO_STAGE, addedToStage);
        }

        private function addedToStage(e:Event):void {
            removeEventListener(Event.ADDED_TO_STAGE, addedToStage);

            stage.addEventListener(MouseEvent.CLICK, click);
        }

        private function click(e:MouseEvent):void {
            var menu:NativeMenu = new NativeMenu();
            for (var i:uint = 0; i < 1169; i++){
                var nativeMenuItem:NativeMenuItem = new NativeMenuItem("Item " + i);
                trace(menu.addItem(nativeMenuItem));//Last item doesn't return null
            }

            try {
                menu.display(stage, e.stageX, e.stageY);//This line cause crash
            } catch (err:Error){
                trace("NativeMenu display error");
            }

        }
    }
}
ajwfrost commented 3 days ago

Yes - sorry, I wasn't clear enough there... the crash is happening because of some implementation error when the OS fails to add the menu item. In terms of the existing error handling, what I meant was that if there is an error condition that the glue logic detects, it returns null i.e. this is what /should/ happen in the scenario where the OS has hit a limit.

In other words: when we provide the fixed version, it should then return null.. currently though it's crashing :-(