rustdesk-server icon indicating copy to clipboard operation
rustdesk-server copied to clipboard

Server in the container does not respond to SIGINT/SIGTERM

Open mini-bomba opened this issue 2 years ago • 34 comments

The server in the docker container does not seem to repond to SIGINT (aka the result of ctrl+c) or SIGTERM (signal used by podman container stop) signals, which doesn't allow me to cleanly shut down the container. This makes it impossible to shut down the container using a simple ctrl+c, and using the stop command takes more time than it should (which sends SIGKILL after a timeout). SIGKILL obviously works (via podman container kill), but it's not a clean exit.

I'm running this on an aarch64 (aka. arm64) system - Ubuntu 22.04 LTS, with podman as a replacement to docker, using a rootless container. I've also tried sending in the signals directly into the container process - no results. The solution I would like to see is a signal handler to be added for those signals, which would cause the process to cleanly exit. I'm concerned that forcibly ending the process via SIGKILL can cause data loss. This also might apply to standalone server executables, but I didn't test those.

mini-bomba avatar Jun 17 '22 19:06 mini-bomba

This issue also applies to docker, with amd64 arch.

paspo avatar Jun 17 '22 22:06 paspo

https://docs.docker.com/engine/reference/commandline/kill/

rustdesk avatar Jun 18 '22 02:06 rustdesk

I'm sorry, @rustdesk, but you're not explaining why it's ok to not have a clean way to exit the main program loop.

BTW, almost every other program in the unix world will perform an exit when receiving a CTRL+C.

paspo avatar Jun 18 '22 07:06 paspo

@rustdesk SIGKILL'ing the process isn't a solution to my issue. Most linux programs support clean exit on SIGINT or at least SIGTERM. Not supporting this causes confusion when users try to shut the process down manually using one of the standard ways. (SIGKILL is usually the last resort here, for misbehaving software) It also will cause delays when docker/podman or systemd try to cleanly shut the process down. (systemd may wait for up to 3 minutes before SIGKILL'ing and continuing the shutdown/reboot) Besides, SIGKILL will kill the program no matter what it's doing - I'm worried that if the server gets this signal when it's in the middle of a database operation, some data may be corrupted and manual intervention may be required to restart the service. It's not good practice to have SIGKILL as the intended way to shut down your software, and I don't want to take any chances. Telling users that SIGKILL is the intended way is like telling users that the x button on your program's window is not a feature you support and they should kill the program using task manager instead.

mini-bomba avatar Jun 18 '22 10:06 mini-bomba

That's the limit of docker. If you run without docker, hbbs/hbbr can respond to SIGNIT/SIGTERM. I had the same question before as you about how to respond to SIGINT in docker container, if you find a way, please let me know. I will share with the others.

rustdesk avatar Jun 18 '22 11:06 rustdesk

I understand your concern. So I suggust running with pm2 in the doc. Or run with system service.

https://github.com/dinger1986/rustdeskinstall

rustdesk avatar Jun 18 '22 11:06 rustdesk

Most docker containers I've seen or made don't have the issue with ignoring SIGINT/SIGTERM. I've also tried sending SIGINT/SIGTERM straight into the container process, which didn't work. I'm also not sure where the source Dockerfile is. I'll do some more testing in the meantime, but the container's source would be useful for debugging.

mini-bomba avatar Jun 18 '22 12:06 mini-bomba

You can try to run below in different way. I do not use docker in my job.

sudo docker run --name hbbs -p 21115:21115 -p 21116:21116 -p 21116:21116/udp -p 21118:21118 -v `pwd`:/root -it --net=host --rm rustdesk/rustdesk-server hbbs -r <relay-server-ip[:port]> 

rustdesk avatar Jun 18 '22 12:06 rustdesk

That's pretty much how I'm running it, with some minor changes that shouldn't cause this issue.

mini-bomba avatar Jun 18 '22 12:06 mini-bomba

I would also appreciate if you reopened the issue, as it is clearly not resolved.

mini-bomba avatar Jun 18 '22 12:06 mini-bomba

Yes, sorry for my misunderstanding.

rustdesk avatar Jun 18 '22 12:06 rustdesk

That's the limit of docker.

I just tested that, and I don't understand how this work.

I explain my test: I'd like to check if a simple rust program, executed inside docker, can be stopped with SIGINT/SIGTERM. Sadly I know zero about rust, but I'm good at googling... So I came up with this:

I created a stupid simple program, which prints an "hello world" and waits forever. Let's call this hello.rs:

use std::{thread, time};

fn main() {
    // Statements here are executed when the compiled binary is called

    // Print text to the console
    println!("Hello World!");

    let ten_millis = time::Duration::from_millis(10);

    while true {
        thread::sleep(ten_millis);
    }
}

I then started a new docker container, with the default rust image, compiled the code, run it and then pressed CTRL+C (SIGINT) and it worked.

docker run --rm --name tttt -ti -v $PWD/src rust:alpine
cd /src
rustc hello.rs
./hello

I checked if I can stop the program with kill: I opened another shell in the same container and tested:

docker exec -ti tttt /bin/sh
ps aux
kill -s 2  [pid_of_hello]   # sigint
kill -s 15  [pid_of_hello]    # sigterm

Everything worked.

I repeated everything with the rust:debian image, just to be on the safe side, with identical results.

I'm now recompiling rustdesk-server in these images to do more tests.

paspo avatar Jun 18 '22 12:06 paspo

I'm now recompiling rustdesk-server in these images to do more tests

Good job, Thanks.

rustdesk avatar Jun 18 '22 13:06 rustdesk

I've done some tests and found the cause.

Test 1: I've compiled rustdesk-server in the rust:latest image (rust:alpine was a pain to get working due to how minimal it is, so I skipped it) I've then ran the hbbr binary and send a SIGINT via CTRL+C. This caused the program to exit, as expected.

Test 2: I've made a docker image simillar to the officially published one. Here's the code:

FROM docker.io/rust:latest
RUN git clone https://github.com/rustdesk/rustdesk-server /tmp/rustdesk-server && cargo build -r --manifest-path /tmp/rustdesk-server/Cargo.toml && cp /tmp/rustdesk-server/target/release/hbbr /tmp/rustdesk-server/target/release/hbbs /usr/bin && rm -rf /tmp/rustdesk-server
WORKDIR /root

I've built the image, tagged it as rustdesk-server-local and ran hbbr using podman run -it --rm rustdesk-server-local hbbr. Then I sent SIGINT into the container via CTRL+C - Program did not exit.

Test 3: I've made a very simple entrypoint script in the same directory as the Dockerfile and made it executable:

#!/bin/bash
$@

The script ran any command provided after the container tag, making it function the same as if it wasn't there. I've also modified the Dockerfile, adding this at the bottom:

COPY entrypoint.sh /entrypoint.sh
ENTRYPOINT ["/entrypoint.sh"]

I've built the image, tagged it as rustdesk-server-local and ran hbbr using podman run -it --rm rustdesk-server-local hbbr. This time when I sent SIGINT via CTRL+C, the program exited.

The difference between tests 2 and 3 is that in test 2 hbbr was running as PID 1 in the container (as if it was the init program), while in test 3 hbbr was running as PID 2, with our entrypoint script being the process with PID 1. See screenshots below: Test 2 Test 3

Processes with PID 1 in linux are a little different when it comes to signals - They don't have the default SIGINT/SIGTERM handlers and are immune to SIGKILL (from inside the same namespace/container - yeah, I forgot to mention that hbbr/hbbs in their current containers are immune to SIGKILL from inside, so you can't kill them at all) That's why when a bash script is the PID 1 process, everything works as intended - bash implements it's own, proper signal handlers. I see 2 solutions here, with the first one being the best.

  1. Implement signal handlers for SIGINT and SIGTERM for hbbr and hbbs. You can do the proper cleanup here (call any cleanup functions of dependencies, close used sockets & files, release used resources, etc.) and then exit the program with the exit code of 0.
  2. Add an entrypoint script to the container image.

While the second option may be quicker to implement, (since I've basically described the whole solution including the code in this comment) I urge you to implement your own, proper signal handlers, since those should prevent further issues with clean exits.

mini-bomba avatar Jun 18 '22 16:06 mini-bomba

@mini-bomba you beated me by a 5 minutes...

Same conclusions, I agree 100%.

Maybe it's time to add an official Dockerfile, maybe with healthcheck support.

paspo avatar Jun 18 '22 16:06 paspo

What about s6-overlay to manage the start/stop of services?

This will imply to have 2 different containers for hbbr & hbbs or a single container with both.

paspo avatar Jun 18 '22 16:06 paspo

@rustdesk @mini-bomba can you take a look at this?

https://github.com/rustdesk/rustdesk-server/pull/40

paspo avatar Jun 20 '22 12:06 paspo

@mini-bomba can you try with the latest images?

docker run --name rustdesk-server \
  -p 21115:21115 -p 21116:21116 -p 21116:21116/udp \
  -p 21117:21117 -p 21118:21118 -p 21119:21119 \
  -e "RELAY=rustdeskrelay.example.com" \
  -v "$PWD/data:/data" -d rustdesk/rustdesk-server-s6:latest

paspo avatar Jul 01 '22 12:07 paspo

@paspo without --net=host, our P2P direct link (via hole punch) can not work, because hbbs can only get virtual ip of docker rathter than real ip of the connection.

rustdesk avatar Jul 01 '22 12:07 rustdesk

That's why we encourage --net=host in the doc.

rustdesk avatar Jul 01 '22 12:07 rustdesk

Another minor issue, If -k _ enabled, your current docker may have problem, because hbbr need to read the public key generated by hbbs, we need to start hbbr a litter later than hbbs.

rustdesk avatar Jul 01 '22 12:07 rustdesk

Another minor issue, If -k _ enabled, your current docker may have problem, because hbbr need to read the public key generated by hbbs, we need to start hbbr a litter later than hbbs.

https://github.com/rustdesk/rustdesk-server/issues/51

paspo avatar Jul 01 '22 13:07 paspo

@paspo without --net=host, our P2P direct link (via hole punch) can not work, because hbbs can only get virtual ip of docker rathter than real ip of the connection.

https://github.com/rustdesk/rustdesk-server/issues/52

paspo avatar Jul 01 '22 13:07 paspo

@mini-bomba can you try with the latest images?

Sorry, I'm currently away for about 2 weeks and can't test this. I also use rootless podman, but I'll figure out the details myself. I've found different --net parameters to use there, which preserve the original IP, but again - I don't have access to my computer right now.

mini-bomba avatar Jul 01 '22 14:07 mini-bomba

@mini-bomba any update on this?

paspo avatar Jul 27 '22 06:07 paspo

@paspo without --net=host, our P2P direct link (via hole punch) can not work, because hbbs can only get virtual ip of docker rathter than real ip of the connection.

You know you could easily check the external up with online IP checkers and then use that for the hole punching. Mysterium VPN does this. However I don't believe their implementation is open source...maybe wrong though.

simeononsecurity avatar Aug 02 '22 11:08 simeononsecurity

Would just like to add that I'm running into the same issues. Unless my pr for the docs is accepted, with the current build and the current docker commands Because -it is specified it runs the container interactively and you can't exit it without opening another terminal and killing the task. I changed it in my pr to -td to background it rather than running interactively. This isn't a limitation of docker and is definitely a limitation of the software imo.

  • source I run and manage hundreds of containers.

simeononsecurity avatar Aug 02 '22 11:08 simeononsecurity

@simeononsecurity please consider testing the alternative rustdesk-server-s6 docker image

it is a single image, with both executables (hbbr and hbbs) running as services, controlled by s6-overlay. Some info in the readme

paspo avatar Aug 02 '22 11:08 paspo

You know you could easily check the external up with online IP checkers and then use that for the hole punching. Mysterium VPN does this. However I don't believe their implementation is open source...maybe wrong though.

This is not the main problem. The issue with bridged networking is that from the point of view of hbbs, every connection is coming from 172.17.0.1, instead of the real IP address.

paspo avatar Aug 02 '22 12:08 paspo

@simeononsecurity please consider testing the alternative rustdesk-server-s6 docker image

it is a single image, with both executables (hbbr and hbbs) running as services, controlled by s6-overlay. Some info in the readme

Is there any real benefit of running the separate images other than spreading the load? Seems to me it might just be better to just use the s6 image all the time then...

simeononsecurity avatar Aug 02 '22 12:08 simeononsecurity