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

Keep alive when i3bar hidden #266

Merged
merged 3 commits into from
Apr 13, 2016
Merged

Keep alive when i3bar hidden #266

merged 3 commits into from
Apr 13, 2016

Conversation

tobes
Copy link
Contributor

@tobes tobes commented Apr 12, 2016

By default i3bar sends a -SIGSTOP to a process sending it data. This causes
some modules interacting with i3 ipc to be suspended, which lead to locking
issues inside i3. This patch switches to using SIGUSR2 as the suspend
signal.

The async modules eg window_title_async were affected.

see #253

@tobes
Copy link
Contributor Author

tobes commented Apr 12, 2016

@cornerman could you just confirm that this is working for you. It should all be good but worth double checking.

By default i3bar sends a -SIGSTOP to a process sending it data.  This causes
some modules interacting with i3 ipc to be suspended, which lead to locking
issues inside i3.  This patch switches to using SIGUSR2 as the suspend
signal.

The async modules eg window_title_async were affected.

see ultrabug#253
@tobes
Copy link
Contributor Author

tobes commented Apr 12, 2016

Note: this patch just fixes the immediate issue of i3 crashing.

Further work is needed.

  • We should put i3status to sleep when we are suspended.
  • We should stop py3status modules too (any that need to stay alive should be using a thread).

@cornerman
Copy link
Contributor

It works for me!

@cornerman
Copy link
Contributor

Wait, this does not work as expected. Sorry, but I overlooked that i3status actually crashes, so we cannot just continue when we receive USR2 from i3status. There is something else going on...

@cornerman
Copy link
Contributor

By default, signals are dispatched to all subprocesses and i3status exits when it receives a SIGUSR2. So, in order to avoid sending this signal to i3status, we could do this when we open the subprocess:

def preexec():
    # Ignore the SIGUSR2 signal for this subprocess
    signal.signal(signal.SIGUSR2, signal.SIG_IGN)

i3status_pipe = Popen(
                ['i3status', '-c', tmpfile.name],
                stdout=PIPE,
                stderr=PIPE,
                preexec_fn=preexec)

We could probably also translate it to SIGSTOP there.

@tobes
Copy link
Contributor Author

tobes commented Apr 12, 2016

Hmm this is frustrating. Yesterday I could reproduce this bug today I can't.

@cornerman is there a minimal i3status.conf that will hit this for you on master?

general {
        output_format = "i3bar"
        colors = true
        interval = 5
}


order += "static_string"
order += "load"
order += "window_title_async"


static_string {
    format = "hello world"
}

window_title_async {
    always_show = True
}

feels like it should be enough but I can't trigger stuff

@cornerman
Copy link
Contributor

is there a minimal i3status.conf that will hit this for you on master?

Seems that it happens with any config for me. What happens if you do:

killall -s USR2 i3status

For me, all i3status processes exit and py3status busy waits as we now ignore that i3status exited.

@cornerman
Copy link
Contributor

This will also fix #179

@tobes
Copy link
Contributor Author

tobes commented Apr 12, 2016

OK, I can trigger it again via i3-msg bar mode hide and i3-msg bar mode dock

Should have an updated patch soon. Thanks

@tobes
Copy link
Contributor Author

tobes commented Apr 12, 2016

@cornerman could you test the latest version which includes your fix and also should stop i3status (which should wake up too)

@cornerman
Copy link
Contributor

Nice, it works!

@tobes
Copy link
Contributor Author

tobes commented Apr 12, 2016

Nice bit of teamwork there 👍

@ultrabug this patch should be ready for consideration now

# If we are 'suspended' then we recieve this
# signal from i3status and can safely ignore
# it.
if code == -SIGUSR2:
Copy link
Owner

Choose a reason for hiding this comment

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

sorry if it should be obvious, but why negate the SIGUSR2 value here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was an odd thing but is no longer needed as we do not send the SIGUSR2 to i3status now. Maybe an i3status bug?

@ultrabug ultrabug added the bug 😞 I am reporting a bug label Apr 13, 2016
@ultrabug ultrabug self-assigned this Apr 13, 2016
@ultrabug
Copy link
Owner

You guys baffle me 👀 quick question and I'll merge. Thanks

def preexec():
# Ignore the SIGUSR2 signal for this subprocess
signal(SIGUSR2, SIG_IGN)

i3status_pipe = Popen(
Copy link
Owner

Choose a reason for hiding this comment

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

why not self.i3status_pipe here directly ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've moved this into the Popen def is that what you meant?

@tobes
Copy link
Contributor Author

tobes commented Apr 13, 2016

I've updated this should be better now

@ultrabug ultrabug merged commit 09ec0aa into ultrabug:master Apr 13, 2016
@ultrabug
Copy link
Owner

Thank you, yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 😞 I am reporting a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants