Skip to content

Commit

Permalink
No need for || exit "$?"
Browse files Browse the repository at this point in the history
Follow-up to #10819

@dumbbell made a note that the `|| exit "$?"` code is probably redundant.

@lukebakken tested this patch using the RMQ docker image and `dash`, and
the test procedure provided by @giner here:

#10819 (comment)

```sh
mkdir ~/ubuntu_24.04
vagrant init ubuntu/noble64
cd ~/ubuntu_24.04
vagrant up
vagrant ssh

sudo apt update
sudo apt install docker.io
newgrp docker

mkdir -p /tmp/rabbitmq
cd /tmp/rabbitmq
cat > Dockerfile << 'EOF'
from "rabbitmq"
RUN sed -i 's/wait "\$rabbitmq_server_pid" || true/wait "$rabbitmq_server_pid" || (exit "$?")/g' "$(which rabbitmq-server)"
EOF
docker build -t rabbitmq-test-script .

docker rm -f rabbitmq-orig 2>/dev/null
docker run --restart on-failure -d --name rabbitmq-orig rabbitmq

docker rm -f rabbitmq-test-script 2>/dev/null
docker run --restart on-failure -d --name rabbitmq-test-script rabbitmq-test-script

sleep 5
docker exec rabbitmq-orig /bin/sh -c 'kill -USR2 $(pidof beam.smp)'
docker exec rabbitmq-test-script /bin/sh -c 'kill -USR2 $(pidof beam.smp)'

sleep 5
echo
echo "*** rabbitmq-orig ***"
docker logs rabbitmq-orig 2>&1 | grep "User defined signal\|Starting RabbitMQ"
echo
echo "*** rabbitmq-test-script ***"
docker logs rabbitmq-test-script 2>&1 | grep "User defined signal\|Starting RabbitMQ"
echo
docker ps -a

```
  • Loading branch information
lukebakken authored and michaelklishin committed Apr 11, 2024
1 parent 7123492 commit 8ba0562
Showing 1 changed file with 6 additions and 17 deletions.
23 changes: 6 additions & 17 deletions deps/rabbit/scripts/rabbitmq-server
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ start_rabbitmq_server() {
stop_rabbitmq_server() {
if test "$rabbitmq_server_pid"; then
kill -TERM "$rabbitmq_server_pid"
wait "$rabbitmq_server_pid" || exit "$?"
wait "$rabbitmq_server_pid"
fi
}

Expand Down Expand Up @@ -129,23 +129,12 @@ else
trap "stop_rabbitmq_server; exit 130" INT

start_rabbitmq_server "$@" &
export rabbitmq_server_pid=$!
export rabbitmq_server_pid="$!"

# Block until RabbitMQ exits or a signal is caught.
# Waits for last command (which is start_rabbitmq_server)
#
# The "|| true" is here to work around an issue with Dash. Normally
# in a Bourne shell, if `wait` is interrupted by a signal, the
# signal handlers defined above are executed and the script
# terminates with the exit code of `wait` (unless the signal handler
# overrides that).
# In the case of Dash, it looks like `set -e` (set at the beginning
# of this script) gets precedence over signal handling. Therefore,
# when `wait` is interrupted, its exit code is non-zero and because
# of `set -e`, the script terminates immediately without running the
# signal handler. To work around this issue, we use "|| true" to
# force that statement to succeed and the signal handler to properly
# execute. Because the statement below has an exit code of 0, the
# signal handler has to restate the expected exit code.
wait "$rabbitmq_server_pid" || exit "$?"
# In a POSIX Bourne shell, if `wait` is interrupted by a signal, the signal
# handlers defined above are executed and the script terminates with the
# exit code of `wait` (unless the signal handler overrides that).
wait "$rabbitmq_server_pid"
fi

0 comments on commit 8ba0562

Please sign in to comment.