Pacman Block fakeroot disposal #30
Comments
|
Hmm, that's really strange. I took a look at it yesterday but for some reason the project wasn't building for me after recent changes. I'm gonna take a better look at it soon. I wonder if there's something you're supposed to call after spawn(). A new compiler warning popped up that complained about a return value not being used after spawn. I suspect that's relevant. |
|
I really don't have time to help you a whole lot right now, but spawn() returns a handle to the child process and doesn't block until it's finished. You should use the handle to ensure all child processes are killed in the end. Also, try to update in a separate thread, that way other block updates don't stall until you've updated the package db. Actually, any update which queries network or does some other huge operation should be executed separately. Should have documented this and told you earlier, sorry about that, but I'm to my neck in other tasks right now. Look at the pending PR for the weather block; the guy did exactly that (his code is very nice). Use an Arc<Mutex> to send data between your update thread and the main thread. |
|
Oh, and no idea why it wasn't building, I tried paying attention not to push broken changes. Just checked and master is building fine, except for a couple warnings. |
|
Thanks for the suggestion :). I don't know what was wrong either, I was getting some errors about missing things. I ended up just removing and recloning the repo and all was well. I'll make the changes you suggested. |
Calling out to a shell is not necessary for a bulk of the commands in `get_update_count`, it probably is bad practice to do so. This commit cleans `get_update_count` up and removes most of the shell-calls. In addition, `run_command` has been changed to wait for the executed commands to exit. `Command::spawn` will spawn the processes without blocking, i.e. the program will continue with it's normal execution immediately, even though the desired result (e.g. the update of the pacman database) is not even completed yet. The `trap` has been removed, since it either wouldn't have worked reliably (since the launched process was not kept track of) or it exited immediately, causing the `rm` call to the pacman lock to execute and delete any lock before the other processes are done. Deleting the pacman lock in case of a failure is a benevolent idea, but would require full error tracking for the pacman related processes. This should fix issue greshake#30.
Calling out to a shell is not necessary for a bulk of the commands in `get_update_count`, it probably is bad practice to do so. This commit cleans `get_update_count` up and removes most of the shell-calls. In addition, `run_command` has been changed to wait for the executed commands to exit. `Command::spawn` will spawn the processes without blocking, i.e. the program will continue with it's normal execution immediately, even though the desired result (e.g. the update of the pacman database) is not even completed yet. The `trap` has been removed, since it either wouldn't have worked reliably (since the launched process was not kept track of) or it exited immediately, causing the `rm` call to the pacman lock to execute and delete any lock before the other processes are done. Deleting the pacman lock in case of a failure is a benevolent idea, but would require full error tracking for the pacman related processes. This should fix issue greshake#30.
|
I'll keep an eye on it. by the way, I just noticed the click isn't working yet (you're not using a ButtonWidget)... Sorry for the big clusterfuck that is widgets and click handling right now.. Will be redone soon. Also, blocks that take a really long time to update currently block other blocks from updating. This will also be fixed with my partial rewrite. |
|
Hi, |
|
@Peltoche are you running an up-to-date version of i3status-rust? This was probably fixed with cb8ac6b, at least I haven't experienced this issue since then. Be careful: if you update to a relatively recent version, you will have to update your configuration from JSON to TOML, see here on how to do it. |
|
@Peltoche I don't think this can be caused by the Music block. It's using a single thread to wait for Dbus signals, and it is only created on startup. Please elaborate if you have further information, 'is present' is not very helpful with a new issue. If the music block also has a thread leak, the issue won't be related to the pacman block. |
|
@pitkley an update fix it indeed, thanks. |
|
Alright, I assume that this is resolved now. Closing |
@keeslinp : The pacman block clearly still has issues with cleaning up after updates. This is a screenshot of the process hierarchie after running for like 2h:
The text was updated successfully, but these errors were encountered: