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

pkill -RTMIN and nohup & don't work correctly #24

Closed
yuandi42 opened this issue Apr 2, 2022 · 13 comments
Closed

pkill -RTMIN and nohup & don't work correctly #24

yuandi42 opened this issue Apr 2, 2022 · 13 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@yuandi42
Copy link

yuandi42 commented Apr 2, 2022

Sorry to bother you again with stupid questions. Below is a clumsy shell script called sb-mpd, written by me to show mpd status:

#!/usr/bin/sh
# show mpd status. WIP

icon=" ------"
color="^c#7c6f64^"
if [ $(pidof mpd) ]; then 
    case $BLOCK_BUTTON in
        1) mpc toggle >/dev/null 2>&1 && pkill -RTMIN+15 dwmblocks;;
        2) notify-send "Current Music" "$(mpc status|head -n1)"
           pkill -RTMIN+15 dwmblocks;; # show current music
        3) nohup setsid -f "$TERMINAL" -e ncmpcpp &;;  # right click, open ncmpcpp
        4) mpc prev >/dev/null 2>&1  ;;  # scroll up, previous
        5) mpc next >/dev/null 2>&1  ;;  # scroll down, next
    esac
    stt=$(mpc status | grep '^\[p'| cut -d " " -f 1)
    case $stt in
        *pause*) icon=" PAUSED";;
        *play*)  icon=" PLYING"; color="^c#427b58^";;
    esac
else 
    case $BLOCK_BUTTON in
        1) mpd && pkill -RTMIN+15 dwmblocks;; # left click to launch mpd
        3) "$TERMINAL" -e "$EDITOR" "$0";; 
    esac
    icon=" NO MPD"
fi

printf "|$color$icon^d^"

There may be some unicode characters display wrongly since I use nerd fonts.

The first problem I had encounter with is in the line 22 mpd && pkill -RTMIN+15 dwmblocks. Ideally when I left click the block, mpd should start then the block should refresh, and it work correctlly when I test the command in my terminal. However, when I try to left click the block, blocks doesn't refresh, still showing "NO MPD" (mpd is actually launched though).

Another problem is about command in the line 11 nohup setsid -f "$TERMINAL" -e ncmpcpp &. In my view, ncmpcpp would be launched independently so that the block can refresh or accept later click event. Well, it doesn't. And htop tree shows that there is a child process echo $(sb-mpd) hanging under dwmblocks.

I would appreciate for your anwser.
P.S.: obviously setsid -f doesn't work either.

@gonviegas
Copy link

gonviegas commented Apr 6, 2022

I've been using dwmblocks with signal and clickable blocks for some time now and the only reason that I'm not using dwmblocks-async is that I have the same problem as well:

kill/pkill signalling dwmblocks isn't working within the scripts used in dwmblocks or if the scripts are called from dwmblocks scripts.

Is there any known workarounds for this to work?

Btw, I use Arch (pun intended).

Thank you.

@apprehensions
Copy link

in line 22, if i'm not correct it's not needed to send a signal to dwmblocks. it should already refresh upon clicking.

@gonviegas
Copy link

gonviegas commented Apr 9, 2022

in line 22, if i'm not correct it's not needed to send a signal to dwmblocks. it should already refresh upon clicking.

Yes, you're correct. No need to signal when clicking or scrolling.

Still, I don't know what's happening cause it refreshes to the second-to-last state, instead of the last one and if I manually send signal through a dwmblock script, it won't just work. But if I send it through terminal, it works.
I have no problem with dwmblocks using the same config, just in dwmblocks-async.

@explosion-mental
Copy link

@yuandi42
setsid -f "$TERMINAL" -e ncmpcpp works fine for me. I can 'use' the block while the terminal is open. If I remove setsid -f, then the block it's, well, blocked (waiting for the task to finish).

About the mpd stuff, I'm not sure. Maybe sleeping a bit before sending a signal to dwmblocks, since by default it won't update.

And about your script I see many room of improvement, took me like 30 mins or more to get my music script done and reading mpc man page. You can use -q instead of sending to dev/null.

@yuandi42
Copy link
Author

@explosion-mental
Thanks! I rewrote the script with your suggestion. However, sleep doesn't work for me. I even tried sleep 60 and the block still refused to refreshed.

setsid -f doesn't work neither in a strange way. When I right click the block, terminal should be launched in a new session. Well, as htop tree in pic below shows, it did. But dwmblocks still blocked with a child process hanging. And for certain the block was frozen and wouldn't refresh. I do wonder why.
1650299888
.

@UtkarshVerma
Copy link
Owner

UtkarshVerma commented Apr 19, 2022

I think this behaviour is related to me wrapping the block command inside and echo:
https://github.com/UtkarshVerma/dwmblocks-async/blob/main/main.c#L15

Therefore, the command "sb-music" actually ends up being executed as "echo $(sb-music)".

The reason why I had to use the echo is because dwmblocks-async executes a block by forking and sending the output back to the parent through pipes. The parent polls for events on these pipes and updates the block whenever something can be read from them. As the parent requires a pipe to be readable to update the corresponding block. The issue arises when a block simply exits without writing anything to the pipe. Therefore, though our block script ran, the parent wouldn't have any idea.
Therefore, I used an echo to wrap the command, so that something is always written to the pipe ("\n" in case of no output from the block).

This was working fine, but this issue is a limitation that has risen with this approach. Sadly, time is not something I have currently to investigate this issue. Therefore PRs and suggestions are highly welcome.

@UtkarshVerma UtkarshVerma added enhancement New feature or request help wanted Extra attention is needed labels Apr 19, 2022
@UtkarshVerma UtkarshVerma pinned this issue Apr 19, 2022
@explosion-mental
Copy link

Oh yeah, forgot to mention I didn't use the BLOCK macro.

My suggestion is to omit the echo macro and make a notice about blocks that don't output anything. In that case anyone could just do sb-printnothing; echo ''. That's what I did

@UtkarshVerma
Copy link
Owner

UtkarshVerma commented Apr 20, 2022

That's a workaround, but ideally, dwmblocks-async shouldn't require it in the first place. Some code reassessment is in order.

@UtkarshVerma
Copy link
Owner

Another workaround is to have programs launched quietly in the script. For example, this prevents the block from being stalled:

st -e ncmpcpp >/dev/null 2>&1 &

@explosion-mental
Copy link

Hey think I found out something more. For blocks that don't output anything the block will never be updated, say a block needs to fetch or wait for something before showing the info, so it exits, not echoing nothing. This is a problem caused by execlock.

Logic; Say sb-blank is a command of some block:

  1. sb-blank doesn't output anything.
  2. dwmblocks execs the command so it locks it with execlock.
  3. epoll doesn't get anything about sb-blank block since it's output it's
    nothing (STDOUT_FILENO for that block is empty).
  4. Now I change sb-blank to echo Hello dwm.
  5. getcmd() won't exec sb-blank because it's execlock bit it's set.

@UtkarshVerma
Copy link
Owner

That's precisely the problem. The execLock mechanism is necessary so we don't run the same command repeatedly. We need a reliable way to detect if a subprocess has exited.

Currently, I just poll the block's pipe to see if any text has been pushed to it. But that doesn't work if the block doesn't output anything, which causes our execLock bit to get stuck as 1 for the specific block.

I've been trying to think of other ways of detecting the exit of forks but nothing has come up.

@UtkarshVerma
Copy link
Owner

I think I have found a fix for this issue finally. It was pretty simple looking back at it. Anyhow, the fix is in the new-org branch. I was playing around with the build system and code organization, so haven't merged it to main yet. But hopefully, it will be merged soon once I figure out the following problems I'm facing with the new organization:

  • I have to define blocks in blocks.c for the time being, which isn't good because everything should ideally be in config.h.
  • I need to find a way for the compiler to dynamically figure out the number of blocks. Currently N_BLOCKS macro has to be defined manually, which isn't good.

@UtkarshVerma
Copy link
Owner

Closing in favour of #45

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants