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

Don't restart the app #45

Closed
ris58h opened this issue Aug 31, 2022 · 8 comments · Fixed by #46
Closed

Don't restart the app #45

ris58h opened this issue Aug 31, 2022 · 8 comments · Fixed by #46
Assignees
Labels
enhancement New feature or request

Comments

@ris58h
Copy link

ris58h commented Aug 31, 2022

I'm curious why do we need to restart the app? It seems like a hack. Could we get rid of it?

@artginzburg
Copy link
Owner

You mean the freeze on wake that happens in some cases, that forces the user to restart the app?

@Filaipus
Copy link

You mean the freeze on wake that happens in some cases, that forces the user to restart the app?

I thought this was expected, this happens to me every single day. Is there a known way to fix this?

@artginzburg
Copy link
Owner

This was not the case in previous versions of macOS. It seems to get progressively harder to restart the app on wake with each upgrade.

There definitely are ways to fix this, since apps like BetterTouchTool somehow are able to do it.

The easiest fix that helped me before is to increase the delay for restart on wake, but it's not a stable solution.

@ris58h
Copy link
Author

ris58h commented Aug 31, 2022

I don't get why the app needs to be restarted at all. In the source code I see scheduleRestart function that is called on some events. For me it seems like a dirty hack, but may be I'm wrong. Why it can't work without it?

@artginzburg
Copy link
Owner

I guess you're right, we could do this without restarting the app. Replacing the contents of restartApp function with [self start]; successfully creates additional listeners so that the app works.

The only thing we need to do is cleanup the old listeners before that: otherwise, the middle click happens twice for one gesture after display reconfiguration or multitouch device addition.

For recreating the listeners on wake from sleep it works fine without any tuning though, because the old listeners just die anyway (I wonder if we can make them persistent).

@ris58h
Copy link
Author

ris58h commented Aug 31, 2022

Thank you for your quick response! I get it now.
I would ask you to keep the issue open so we could fix it in the future.

@artginzburg
Copy link
Owner

There is one more problem. If the listeners are restarted before the system fully wakes up, that is, starts the multitouch devices, then the listeners are going to be attached to old instances of multitouch controllers (the terminology might be inaccurate, but it describes the problem good enough), forcing the user to manually restart the app to make it work. This has been an issue for ages, and changing the method of restarting the app doesn't solve it.

Restart the listeners early — the functionality dies, late — the user is forced to wait before being able to use the functionality.

Just thought of a possible solution:
Now that we're able to track the app state continuously, it's possible to make a flag in code that is set to true on system wake. This flag being true will make the listeners be recreated e.g. every 5 seconds. Then in the beginning of touchCallback() and mouseCallback(), the flag will be set to false, since any of these callbacks firing mean that the listeners attached successfully. This might just work for every single system, no matter how fast it starts on wake.

@artginzburg
Copy link
Owner

It seems like I just uncovered (and, very likely, introduced) some memory leaks, but it works — the app does not need to restart itself, and restarts the listeners instead.

artginzburg added a commit that referenced this issue Sep 1, 2022
@artginzburg artginzburg added the enhancement New feature or request label Sep 1, 2022
@artginzburg artginzburg self-assigned this Sep 1, 2022
@artginzburg artginzburg linked a pull request Sep 4, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants