ElektraInitiative / opensesame

3 stars 5 forks source link

signals only accepted once? #108

Open markus2330 opened 10 months ago

markus2330 commented 10 months ago

I have in the config:

ping.enable = 1

Then I do:

sudo killall -SIGUSR1 opensesame

Expected: that I see some output in NC what is going on (ping).

Actual result: no output.

fel115 commented 10 months ago

In my setup, it works.

nextcloud.chat = "<token>"
nextcloud.chat.licht = "<token>"
nextcloud.chat.ping = "<token>"
nextcloud.chat.commands = "<token>"
nextcloud.user = "<user>"
nextcloud.pass = "<pass>"
nextcloud.url = "https://nextcloud.markus-raab.org/nextcloud"
nextcloud.format.datetime = "%d.%m.%Y %H:%M:%S"
nextcloud.format.time = "%H:%M:%S"
validator.user1 = "14, 15 ,13 ,15, 11, 15, 7, 15"
validator.user2 = "14, 12, 14, 15, 11, 15"
ping.enable = 1
markus2330 commented 10 months ago

Did you try with master or with #112?

I retried with https://build.libelektra.org/job/opensesame/job/master/lastSuccessfulBuild/artifact/target/armv7-unknown-linux-gnueabihf/debian/opensesame_0.8.0_armhf.deb on two computers, no success.

Which output do you get with sudo killall -SIGUSR1 opensesame?

Output of sudo killall -SIGHUP opensesame works initially, "reloading config&state for opensesame 0.8.0 19.10.2023 10:59:27" but not again later.

So maybe the problem is that the signal is only accepted once?

markus2330 commented 10 months ago

it seems to be related to sending several signals of different types. E.g. if I send killall -SIGALRM opensesame several times, it works. But if I send, e.g. killall -SIGHUP opensesame in between, killall -SIGALRM opensesame stops working.

fel115 commented 10 months ago

The sighup causes a panic in line 31 of config.rs. https://github.com/ElektraInitiative/opensesame/blob/master/src/config.rs#L31

And I think after the panic the signals task may stop to work. Or have you encountered the problem also with other signals?

So we need to change the use of config.sync and state.sync.

markus2330 commented 10 months ago

I think you are right, I just found in the log:

Okt 19 20:34:57 garage opensesame[15198]: thread 'tokio-runtime-worker' panicked at 'Set config failed: Sorry, module  issued error :
Okt 19 20:34:57 garage opensesame[15198]: : ', src/config.rs:31:17
Okt 19 20:34:57 garage opensesame[15198]: note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I created a bug report for panic not going to Nextcloud: #122

And I don't think we should kill the signal module just because we were not able to handle one signal. So the panics should be removed from the module.

fel115 commented 10 months ago

Yes, this shouldn't happen. But we also need to think about how we want to use the config and state in Environment and Singnals for remember/restore baseline and the sync. I think the sync could be done in the config module itself, but for the remember/restore baseline (and set alarm), I don't know if we can use this with channels.