UtkarshVerma / dwmblocks-async

An efficient, lean, and asynchronous status feed generator for dwm.
GNU General Public License v2.0
224 stars 87 forks source link

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

Closed yuandi42 closed 1 year ago

yuandi42 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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.

explosion-mental commented 2 years ago

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 commented 2 years ago

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

UtkarshVerma commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

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:

UtkarshVerma commented 1 year ago

Closing in favour of #45