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

Very rare crashes in SIApplication observerCallback #10

Open
ianfixes opened this issue Oct 24, 2017 · 15 comments
Open

Very rare crashes in SIApplication observerCallback #10

ianfixes opened this issue Oct 24, 2017 · 15 comments

Comments

@ianfixes
Copy link
Contributor

ianfixes commented Oct 24, 2017

void observerCallback(AXObserverRef observer, AXUIElementRef element, CFStringRef notification, void *refcon) {
    SIAXNotificationHandler callback = (__bridge SIAXNotificationHandler)refcon;
    SIWindow *window = [[SIWindow alloc] initWithAXElement:element];
    callback(window);
}

(__bridge SIAXNotificationHandler)refcon fails with Thread 1: EXC_BAD_ACCESS (code=1, address=0x60000b000000)

I've noticed this only very rarely, and it might be due to the "what's new" dialog in MS Outlook.

@ianfixes
Copy link
Contributor Author

ianfixes commented Jan 5, 2018

More info after a crash

(lldb) po window
<c0d06200 80610000 00000000 00000000>

(lldb) po observer
<AXObserver 0x61800089d0b0> {pid=52339}

(lldb) po element
<AXUIElement 0x60002b645c40> {pid=52339}

(lldb) po notification
AXUIElementDestroyed

(lldb) po refcon
<AXUIElement 0x60002b45e330> {pid=52339}

(lldb) po callback 
0x0000000000000010

(lldb) 

This no longer appears to be Application-specific

@ianfixes ianfixes changed the title Very rare crashes in SIApplication observerCallback, possibly related to opening Outlook Very rare crashes in SIApplication observerCallback Jan 9, 2018
@ianfixes
Copy link
Contributor Author

ianfixes commented Jan 9, 2018

Crashed again today at/around the time that the system preferences window was closed. I've wrapped the entire Silica function in a try/catch block (with breakpoint) to see if just ignoring the error is a viable strategy. Will report back.

@ianfixes
Copy link
Contributor Author

ianfixes commented Feb 20, 2018

Now getting a crash on callback(window);

  • po SIAXNotificationHandler callback = (__bridge SIAXNotificationHandler)refcon; works in the debugger.
  • po window produces <SIWindow: 0x600000025f60> <Title: (null)> <pid: 29810>
  • po callback(nil) produces EXC_BAD_ACCESS

This suggests that something is going hopelessly wrong, sometimes, when getting that callback function from the bridge. Not sure how to troubleshoot further.

Crash was preceded by a lot of windows swapping positions at random intervals based on click events, but on both monitors instead of just the one where the click happened.

@ianfixes
Copy link
Contributor Author

ianfixes commented Mar 7, 2018

Crashed today.

The callback itself seemed to be null.

(lldb) po callback
0x0000000000000008

Is 0x0000000000000008 a value that can be filtered for, and an early exit taken?

@ianfixes
Copy link
Contributor Author

Perhaps there is a race condition where the callback is not available immediately?

(lldb) po callback
0x0000000000000008

(lldb) po (__bridge SIAXNotificationHandler)refcon
0x0000001000000002

Given that

SIAXNotificationHandler callback = (__bridge SIAXNotificationHandler)refcon;

@mutantcornholio
Copy link

I was trying to find why Amethyst crashes sometime, stumbled upon this crash too.
Can we do something about it?

cc/ @ianyh

@ianfixes
Copy link
Contributor Author

ianfixes commented Jul 3, 2018

Crashed while quitting Chrome (which caused the closing of a LOT of Chrome windows).

callback = 0x0000000000000008

@ianyh
Copy link
Owner

ianyh commented Jul 3, 2018

One thought I had with this was to use the entire observer object as the referenced object rather than the block itself.

@ianfixes
Copy link
Contributor Author

ianfixes commented Jul 5, 2018

I don't know enough about ObjC to do the equivalent of guard let callback = callback else {return}, but that seems like what's needed here.

@ianfixes
Copy link
Contributor Author

I'm not sure if this can help reproduce the issue, but I got a crash just now that had to do with maximizing a chrome window by double-clicking the title bar (accidentally). There was a noticeable lag, and I had time to restore the window to proper size... I think it's something to do with trying to act on the maximized window after I've un-maximized, or a similar case.

Is this an indication that some part of the window management changes when you switch between these 2 modes?

@ianyh
Copy link
Owner

ianyh commented Feb 23, 2020

I just accidentally trapped this in a debugger!

observer	AXObserverRef	0x600002178c30	0x0000600002178c30
element	AXUIElementRef	0x600000e59c50	0x0000600000e59c50
notification	CFStringRef	@"AXUIElementDestroyed"	0x0000600000e59fb0
refcon	void *	0x600000e5b480	0x0000600000e5b480
callback	SIAXNotificationHandler	0x0000000000000008	

@ianfixes
Copy link
Contributor Author

🎉 any idea why the callback is an integer with a low-but-not-zero value?

@gerasimsergey
Copy link

may be
// https://github.com/koekeishiya/kwm/issues/511
// #10
if (window && callback && window.app)
callback(window);

@koekeishiya
Copy link

koekeishiya commented Apr 26, 2020

I don't know exactly how the subscription model in Amethyst works, but this looks like a data race error to me. Anyway, I based my model off of observations from running Steam and interactions with its windows. Primarily the Store, Library and Community tabs, as when you hover them they create a new window which is destroyed when the mouse leaves the window. It was easy to reproduce crashes by rapidly moving the mouse in and out of these menus.

By the looks of the trace above this matches the symtoms I described; might be worth investigating.

I fixed this issue by passing in the address of a pointer on my window structure (which is allocated on the heap), that points to memory on the heap, storing the window id. I then invalidate this value when a UIElementDestroyed notification is received, and other unprocessed events check this value before trying to process the event. I have not had any crashes related to the accessibility API after this system was in place.

I see that Amethyst passes a callback to handle the event, rather than information about the entity the event acts upon, so I'm not sure what a suitable fix would be here.

@gerasimsergey
Copy link

gerasimsergey commented Apr 26, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants