Skip to content

Add improved sigint handling for cancellation #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
midnightveil wants to merge 1 commit into from
Closed

Add improved sigint handling for cancellation #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions scripts/common
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,21 @@ HOST=tftp.keg.cse.unsw.edu.au
# Config file location
CONFIG=~/.mq

# Bash implements the "Wait and Cooperative Exit" protocol
# If, when we receive a trap like SIGINT, we do not then exit because of a SIGINT,
# bash assumes that we have handled SIGINT and continues as if we did not try
# to just stop the program execution, making it look like nothing happened.
# Hence, we must emulate this behaviour for bash to believe that we have died
# due to the SIGINT (i.e. that after the wait() syscall, WTERMSIG(status) is SIGINT).
# Ref: https://www.cons.org/cracauer/sigint.html
# Ref: https://mywiki.wooledge.org/SignalTrap#Special_Note_On_SIGINT_and_SIGQUIT
TrapExitSigInt() {
# Restore default handling for SIGINT
trap - SIGINT
kill -s INT $$
}


# Add all the other bits and pieces

. "${SCRIPT_PATH}/scripts/remote"
Expand Down
14 changes: 12 additions & 2 deletions scripts/enqueue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ Enqueue() {

# We can setup the trap early as we check if we actually own the
# the lock before releasing it
trap "UnlockSystem \"${system}\" 0 \"${key}\"; exit 1" ${SIGNALS}
trap "UnlockSystem \"${system}\" 0 \"${key}\"; TrapExitSigInt" ${SIGNALS}

if ! LockSystem "${system}" "${retry_period}" "${total_retries}" "${key}"; then
echo "Failed to acquire lock for system (${system})"
Expand All @@ -257,11 +257,21 @@ Enqueue() {
# Ditto for 'bitstreamflag'
SystemRunImage "${system}" "${completion}" "${completion_timeout}" "${errortxt}" "${logfile}" "${keep_alive}" ${linux} ${dtbflag} ${bitstreamflag} $files
ret=$?

# Special case: SSH does not return indicating a SIGINT, so we special
# case instead that it returns 255 in this case.
# https://github.com/openssh/openssh-portable/blob/V_10_0_P1/mux.c#L2057-L2059
# (However, sometimes it will instead forward through status 130 from the process
# which received SIGINT on the other end of the SSH session. Sometimes...)
if [ "${ret}" == "255" ]; then
Copy link
Copy Markdown
Contributor Author

@midnightveil midnightveil Apr 23, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so this doesn't seem to be reliable.

Our console scripts in /tftpboot will return -1 if various subcommands fail, which results in sys.exit(-1) which terminates python with code 255.

This seems like it would be a bad idea.

I'm going to go with Indan's suggestion: seL4/ci-actions#461 (comment)

TrapExitSigInt
fi
fi

if ! ${no_lock_mods} ; then
UnlockSystem "${system}" 0 "${key}"
trap "exit 1" ${SIGNALS}
# Reset trap behaviour to default.
trap - ${SIGNALS}
fi

exit $ret
Expand Down