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

eframe: Only call run_return twice on Windows #3053

Merged
merged 1 commit into from
Jun 5, 2023

Conversation

pan93412
Copy link
Contributor

@pan93412 pan93412 commented Jun 4, 2023

The approach of #1889 may remove observers in a view twice, which produces the Obj-C Exception:

Cannot remove an observer <...> for the key path "nextResponder" from <WinitView ...> because it is not registered as an observer.

The above message can only be seen when attaching the application to debugger. Users normally see:

[1]    *** trace trap  cargo run

This commit fixes it by only running event_loop.run_return() twice on Windows. Besides:

  • We have set ControlFlow::Exit on Event::LoopDestroyed, EventResult::Exit and on error; therefore, it is safe to not calling set_exit().
  • This commit also fix the persistence function in macOS. It can't store the content in Memory due to this exception.

Fixed: #2768 (eframe: "App quit unexpectedly" on macOS)

Exception Details
2023-06-04 13:01:02.034869+0800 review-tool[31446:18023641] [Window] Warning: Window WinitWindow 0x102536e60 ordered front from a non-active application and may order beneath the active application's windows.
2023-06-04T05:01:07.146383Z DEBUG eframe::native::epi_integration: Received WindowEvent::CloseRequested    
2023-06-04T05:01:07.146439Z DEBUG eframe::native::epi_integration: App::on_close_event returned true    
2023-06-04T05:01:07.146456Z DEBUG eframe::native::run: Asking to exit event loop…    
2023-06-04T05:01:07.241007Z DEBUG eframe::native::run: Received Event::LoopDestroyed - saving app state…    
2023-06-04T05:01:07.241041Z DEBUG eframe::native::run: eframe window closed    
2023-06-04 13:01:34.963635+0800 review-tool[31446:18023641] [General] Cannot remove an observer <_NSTouchBarFinderObservation 0x10247fbb0> for the key path "nextResponder" from  because it is not registered as an observer.
2023-06-04 13:01:35.012630+0800 review-tool[31446:18023641] [General] (
        0   CoreFoundation                      0x00000001a1953154 __exceptionPreprocess + 176
        1   libobjc.A.dylib                     0x00000001a14724d4 objc_exception_throw + 60
        2   Foundation                          0x00000001a2846ff0 -[NSObject(NSKeyValueObserverRegistration) _removeObserver:forProperty:] + 628
        3   Foundation                          0x00000001a2846d28 -[NSObject(NSKeyValueObserverRegistration) removeObserver:forKeyPath:] + 136
        4   Foundation                          0x00000001a28469f4 -[NSObject(NSKeyValueObserverRegistration) removeObserver:forKeyPath:context:] + 196
        5   AppKit                              0x00000001a50b2e90 -[_NSTouchBarFinderObservation invalidate] + 264
        6   AppKit                              0x00000001a50b41f0 ___NSTouchBarFinderSetNeedsUpdateOnMain_block_invoke_2 + 628
        7   AppKit                              0x00000001a4ba0c30 NSDisplayCycleObserverInvoke + 168
        8   AppKit                              0x00000001a4ba088c NSDisplayCycleFlush + 644
        9   QuartzCore                          0x00000001a8dbdce4 _ZN2CA11Transaction19run_commit_handlersE18CATransactionPhase + 120
        10  QuartzCore                          0x00000001a8dbcaa0 _ZN2CA11Transaction6commitEv + 320
        11  AppKit                              0x00000001a4c22c7c __62+[CATransaction(NSCATransaction) NS_setFlushesWithDisplayLink]_block_invoke + 272
        12  AppKit                              0x00000001a52fef7c ___NSRunLoopObserverCreateWithHandler_block_invoke + 64
        13  CoreFoundation                      0x00000001a18d99f0 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 36
        14  CoreFoundation                      0x00000001a18d98dc __CFRunLoopDoObservers + 532
        15  CoreFoundation                      0x00000001a18d8f14 __CFRunLoopRun + 776
        16  CoreFoundation                      0x00000001a18d84b8 CFRunLoopRunSpecific + 612
        17  HIToolbox                           0x00000001ab122c40 RunCurrentEventLoopInMode + 292
        18  HIToolbox                           0x00000001ab1228d0 ReceiveNextEventCommon + 220
        19  HIToolbox                           0x00000001ab1227d4 _BlockUntilNextEventMatchingListInModeWithFilter + 76
        20  AppKit                              0x00000001a4af9d44 _DPSNextEvent + 636
        21  AppKit                              0x00000001a4af8ee0 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 716
        22  AppKit                              0x00000001a4aed344 -[NSApplication run] + 464
        23  review-tool                         0x00000001003b3e74 _ZN61_$LT$$LP$$RP$$u20$as$u20$objc2..message..MessageArguments$GT$8__invoke17he1b0068f4ccdd175E + 52
        24  review-tool                         0x00000001003ad79c _ZN5objc27message8platform15send_unverified28_$u7b$$u7b$closure$u7d$$u7d$17hb7029ac0f15e9fc8E + 40
        25  review-tool                         0x00000001003a74cc _ZN115_$LT$core..panic..unwind_safe..AssertUnwindSafe$LT$F$GT$$u20$as$u20$core..ops..function..FnOnce$LT$$LP$$RP$$GT$$GT$9call_once17hfc492e0857bda5aaE + 40
        26  review-tool                         0x00000001003a3e04 _ZN5objc29exception5catch28_$u7b$$u7b$closure$u7d$$u7d$17h0ba94ebdb44d74d0E + 44
        27  review-tool                         0x00000001003a22f4 _ZN5objc29exception10try_no_ret24try_objc_execute_closure17ha01724ca680604beE + 76
        28  review-tool                         0x00000001003c429c rust_objc_sys_0_2_try_catch_exception + 36
        29  review-tool                         0x00000001003a07ac _ZN5objc29exception10try_no_ret17h7ffc4f7f826f0b25E + 192
        30  review-tool                         0x00000001003a3cec _ZN5objc29exception5catch17hf8d41ca8a06b1454E + 72
        31  review-tool                         0x00000001003b1c50 _ZN5objc27message15conditional_try17h6aa27d5efbaaf4baE + 52
        32  review-tool                         0x00000001003acfec _ZN5objc27message8platform15send_unverified17hb5c1345ce99f3ed5E + 80
        33  review-tool                         0x0000000100371640 _ZN5objc27message15MessageReceiver12send_message17h848695ab0cd168f7E + 384
        34  review-tool                         0x0000000100374150 _ZN5winit13platform_impl8platform6appkit11application13NSApplication3run17h6c17518f765dd74aE + 76
        35  review-tool                         0x0000000100152a84 _ZN5winit13platform_impl8platform10event_loop18EventLoop$LT$T$GT$10run_return28_$u7b$$u7b$closure$u7d$$u7d$17h03b98465f5880858E + 344
        36  review-tool                         0x000000010015cc50 _ZN5objc22rc11autorelease15autoreleasepool17h21343e13671112ecE + 132
        37  review-tool                         0x000000010015274c _ZN5winit13platform_impl8platform10event_loop18EventLoop$LT$T$GT$10run_return17h14e950aef9284697E + 252
        38  review-tool                         0x0000000100156898 _ZN108_$LT$winit..event_loop..EventLoop$LT$T$GT$$u20$as$u20$winit..platform..run_return..EventLoopExtRunReturn$GT$10run_return17h48966b4c910ded19E + 24
        39  review-tool                         0x00000001001595ac _ZN6eframe6native3run14run_and_return17h506c088691f93430E + 644
        40  review-tool                         0x0000000100167c08 _ZN6eframe6native3run16glow_integration8run_glow28_$u7b$$u7b$closure$u7d$$u7d$17ha2f679311e869152E + 112
        41  review-tool                         0x0000000100159284 _ZN6eframe6native3run15with_event_loop28_$u7b$$u7b$closure$u7d$$u7d$17h5cd8d569d08431afE + 328
        42  review-tool                         0x000000010014a7dc _ZN3std6thread5local17LocalKey$LT$T$GT$8try_with17h20e8fd7b588891fbE + 368
        43  review-tool                         0x000000010014a2dc _ZN3std6thread5local17LocalKey$LT$T$GT$4with17h246bd912206bfd9dE + 36
        44  review-tool                         0x0000000100159130 _ZN6eframe6native3run15with_event_loop17h83ef53edb3b5d8e5E + 96
        45  review-tool                         0x0000000100167a4c _ZN6eframe6native3run16glow_integration8run_glow17h13efab21927196e5E + 180
        46  review-tool                         0x0000000100178150 _ZN6eframe10run_native17h66eb1ea1fb564bb8E + 444
        47  review-tool                         0x0000000100011c00 _ZN11review_tool4main17h1cb3b8d076b76f8fE + 944
        48  review-tool                         0x0000000100006aa0 _ZN4core3ops8function6FnOnce9call_once17h4b691ff2c5ef1de1E + 20
        49  review-tool                         0x0000000100002dd4 _ZN3std10sys_common9backtrace28__rust_begin_short_backtrace17h2ee91cdb29921988E + 24
        50  review-tool                         0x000000010001b8b8 _ZN3std2rt10lang_start28_$u7b$$u7b$closure$u7d$$u7d$17h165a9106e93a887cE + 28
        51  review-tool                         0x0000000100624118 _ZN3std2rt19lang_start_internal17hd0038fc73979f110E + 648
        52  review-tool                         0x000000010001b884 _ZN3std2rt10lang_start17hc2f7452d664ffbaaE + 84
        53  review-tool                         0x0000000100011c98 main + 36
        54  dyld                                0x00000001a14a3f28 start + 2236
)

The approach of emilk#1889 may remove observers in a view
twice, which produces the Obj-C Exception:

    Cannot remove an observer <...> for the key path
    "nextResponder" from <WinitView ...> because
    it is not registered as an observer.

The above message can only be seen when attaching the
application to debugger. Users normally see:

    [1]    *** trace trap  cargo run

This commit fixes it by only running `event_loop.run_return()`
twice on Windows. Besides:

* We have set `ControlFlow::Exit` on `Event::LoopDestroyed`,
  `EventResult::Exit` and on error; therefore, it is safe
  to not calling `set_exit()`.
* This commit also fix the persistence function in macOS.
  It can't store the content in Memory due to this exception.

Fixed: emilk#2768 (eframe: "App quit unexpectedly" on macOS)
Signed-off-by: pan93412 <[email protected]>
@pan93412 pan93412 force-pushed the fix-observer-removal-issue branch from 09c0e2a to e472929 Compare June 4, 2023 06:03
@pan93412 pan93412 marked this pull request as ready for review June 4, 2023 06:09
pan93412 added a commit to pan93412/review-tool that referenced this pull request Jun 4, 2023
@emilk emilk added the eframe Relates to epi and eframe label Jun 5, 2023
Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense - thanks!

@emilk emilk merged commit 860dac6 into emilk:master Jun 5, 2023
@pan93412 pan93412 deleted the fix-observer-removal-issue branch June 5, 2023 12:59
@emilk emilk changed the title eframe: Only run_return twice on Windows eframe: Only call run_return twice on Windows Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
eframe Relates to epi and eframe
Projects
None yet
Development

Successfully merging this pull request may close these issues.

eframe: "App quit unexpectedly" on macOS
2 participants