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

Fix crash in smartpause #582

Merged
merged 2 commits into from
Jun 19, 2024

Conversation

deltragon
Copy link
Collaborator

Fixes #580.

by adding missing parameter
This was missed in 903d407
This broke when the first parameter was None, but the second wasn't (the
second parameter wasn't passed at all.)
Use *args/**kwargs to make it behave properly in all cases.
Comment on lines -74 to +75
self.context['api']['disable_safeeyes'] = lambda status: utility.execute_main_thread(
self.disable_safeeyes, status)
self.context['api']['disable_safeeyes'] = lambda status=None, is_resting=False: utility.execute_main_thread(
self.disable_safeeyes, status, is_resting)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the change that fixes the crash - when the lambda is called with the second parameter, python throws an error, as it only takes one parameter.

def execute_main_thread(target_function, arg1=None, arg2=None):
def execute_main_thread(target_function, *args, **kwargs):
"""
Execute the given function in main thread.
"""
if arg1 is not None and arg2 is not None:
GLib.idle_add(lambda: target_function(arg1, arg2))
elif arg1 is not None:
GLib.idle_add(lambda: target_function(arg1))
elif arg2 is not None:
GLib.idle_add(lambda: target_function(arg2))
else:
GLib.idle_add(target_function)

GLib.idle_add(lambda: target_function(*args, **kwargs))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change is technically not necessary to fix the crash - however, the smartpause plugin calls disable_safeeyes(None, True). With the previous code, this would see that arg1 is None - and pass arg2 to the first argument, instead.
This lead to SafeEyes being "manually" disabled, meaning it would not be re-enabled when the system was no longer idle, only when the user clicked "Enable Safe Eyes".

In theory, this could break any caller of execute_main_thread that relied on the older, less intuitive behaviour.
However, disable_safeeyes is the only call that uses the second argument. enable_safeeyes has a second argument, but that is not used anywhere.

@deltragon deltragon changed the title Fix smartpause signature Fix crash in smartpause Jun 10, 2024
Copy link
Contributor

@hartwork hartwork left a comment

Choose a reason for hiding this comment

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

@deltragon looks great!

CC @slgobinath

@slgobinath slgobinath merged commit 4c43de8 into slgobinath:master Jun 19, 2024
1 check passed
@slgobinath
Copy link
Owner

Thanks @deltragon and @hartwork

@slgobinath
Copy link
Owner

@hartwork Safe Eyes 2.1.9 is released with this fix

@deltragon deltragon deleted the fix-smartpause-signature branch June 19, 2024 08:24
@hartwork
Copy link
Contributor

@slgobinath thank you! 🙏

@archisman-panigrahi
Copy link
Collaborator

Given the large number of PRs and open issues, I am somewhat confused about the state of this - does smart pause work with Wayland in version 2.1.9?

@deltragon
Copy link
Collaborator Author

@archisman-panigrahi On GNOME and Sway it works on Wayland, yes. On other compositors (like KDE) not really - it uses xprintidle, which effectively means it sees the system as always idle (unless you're currently interacting with an XWayland app).

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