Page MenuHomePhabricator

all python scripts need sigterm and sigint traps
Closed, ResolvedPublic

Description

Problem, when you start something like

/usr/lib/msgcollector/msgdispatcher_dispatch_x info "test title" "test msg" 1 ""

from terminal, you cannot terminate using ctrl + c. This makes it difficult for calling scripts to terminate them (in case those would receive a signal themselves) and then that process could either hang or stay open. Not critical, but an issue we can fix.

To find them.

grep -r \#\!/usr/bin/python *

What do do... I suggest this.

  • on sigterm -> "Signal sigterm received! Exiting." -> exit 143
  • on sigint (ctrl + c) -> "Signal sigint received! Exiting." -> exit 130

Ideally we have some small code snippet that we can easily reuse and copy and paste into all the scripts.

Details

Impact
Needs Triage

Event Timeline

Patrick created this task.Jan 26 2015, 11:05 PM
Patrick raised the priority of this task from to Normal.
Patrick updated the task description. (Show Details)
Patrick added projects: Whonix 10, python.
Patrick added a subscriber: Patrick.

Ideally, we could set up one global signal trap per script. In most cases we don't need any cleanup code. Or? By "global signal trap", I mean just to set it up once at the very top. Not below in every function and loop and so forth.

I failed to setup a working global trap for the Qt script. Maybe you have any ideas, @troubadour?

We could put this at the top of every script.

#!/usr/bin/python

import signal
import os, sys
import time

f_name = os.path.basename(__file__) + '.pid'
f = open(f_name, 'w')
f.write(str(os.getpid()) + '\n')
f.close()

def receive_signal(signum, signal):
    #print signum
    if signum == 15:    # SIGTERM
        sys.exit(143)
    elif signum == 2:   # SIGINT
        sys.exit(130)

signal.signal(signal.SIGTERM, receive_signal)
signal.signal(signal.SIGINT, receive_signal)

# for testing
while True:
    print 'Waiting...'
    time.sleep(5)

A bash script (!!) for testing (implies that both scripts are in the same directory).

#!/bin/bash

pid=$(cat kill_signal.pid)
kill -SIGINT $pid  # replace with any signal

But that is may be overkill for the graphical scripts (Qt). A more typical use is with daemons.

The above does not work once the Qt event handler loop is running. There is a hack for SIGINT (Ctrl+C), when the script is run from command line. Two lines at the very beginning of the script.

#!/usr/bin/python
# -*- coding: utf-8 -*-
import signal
signal.signal(signal.SIGINT, signal.SIG_DFL)

Don't know if it allows custom traps.
Tested with generic_gui_message, msgdispatcher_dispatch_xand whonix-setup-wizard

Let's find some small code easy solution for the simple non-QT command line tools and then worry about the QT tools later?

Tried your above code. It works when using the sleep testing code. But it doesn't work when connection really hangs (such as tor bootstrap).

Test case, theoretical, bad firewall rules. Practical simulation, on the gateway: Put exit 0 below iptables -t mangle -X in /usr/bin/whonix_firewall, then reload it, sudo whonix_firewall..

On the workstation, see the following test script.

#/bin/bash
set -x
/usr/lib/anon-shared-helper-scripts/tor_bootstrap_check.py 127.0.0.1 9151 0 &
pid="$!"
sleep 1
kill -sigterm "$pid"
wait "$pid"
true "exit code: $?"

Output:

++ pid=30266
++ sleep 1
++ /usr/lib/anon-shared-helper-scripts/tor_bootstrap_check.py 127.0.0.1 9151 0
++ kill -sigterm 30266
++ wait 30266

(The sleep 1 is used to interrupt python during the connection phase, not very early during initialization where sigterm does still function. The wait indicates, that the process is still running, even though signal sigterm was sent.)

Patrick edited projects, added Whonix 11; removed Whonix 10.Feb 12 2015, 12:04 PM
Patrick set Impact to Needs Triage.
Patrick added subscribers: HulaHoop, nrgaway, marmarek.
hqi added a subscriber: hqi.Sep 3 2015, 2:15 AM

I tried this command:
/usr/lib/msgcollector/msgdispatcher_dispatch_x
and found the output is like this:
user@host:~$ /usr/lib/msgcollector/msgdispatcher_dispatch_x
Traceback (most recent call last):

File "/usr/lib/msgcollector/msgdispatcher_dispatch_x", line 91, in <module>
  if str(sys.argv[1]) == "info":

IndexError: list index out of range

So I added some code between '#---' to avoid it:
if name == "main":

import sys
  1. if len(sys.argv) == 1: sys.exit("'msgdispatcher_dispatch_x'. There is no option") #------------------------------------------------- app = QtGui.QApplication(sys.argv) Dialog = QtGui.QDialog()

Try this.

/usr/lib/msgcollector/msgdispatcher_dispatch_x "info" "test title" "test msg" 0 "/path/to/optional/icon"
hqi added a comment.Sep 3 2015, 4:45 PM

there is no problem with options, but I think we may need to cover the scenario if there is no opton

Yes.

Please provide a git branch implementing this. Or a github pull request.

github repository location:

https://github.com/Whonix/msgcollector

https://github.com/Whonix/msgcollector/blob/master/usr/lib/msgcollector/msgdispatcher_dispatch_x

hqi added a comment.Sep 4 2015, 1:15 AM

So any better error message can be put ?
"sys.exit("'msgdispatcher_dispatch_x'. There is no option")"

Exit non-zero, write to stderr and explain the syntax.

https://github.com/joysn/msgcollector/blob/master/usr/lib/msgcollector/msgdispatcher_dispatch_x

Is this what is required here?

Error message for incorrect # of arguments

'msgdispatcher_dispatch_x'. Invalid number of options
msgdispatcher_dispatch_x requires 4 manadatory and 1 optional arguments

  1. Message Type = info|warning|error
  2. Title of the message box
  3. Message in the message box
  4. Position of the message (Value 0 or more)
  5. Icon to be used (optional)

Working commands
/usr/lib/msgcollector/msgdispatcher_dispatch_x "info" "test title" "test msg" 0
usr/lib/msgcollector/msgdispatcher_dispatch_x "info" "test title" "test msg" 0 "/path/to/optional/icon"

If there are lesser arguments, we will see the error.

No. However, that's good to have.

What is missing here... When you run:

/usr/lib/msgcollector/msgdispatcher_dispatch_x info "test title" "test msg" 1 ""

And then press ctrl + c or ctrl + d, nothing happens. The sigterm / sigint signal is ignored. The wanted behavior is to exit.

  • on sigterm -> "Signal sigterm received! Exiting." -> exit 143
  • on sigint (ctrl + c) -> "Signal sigint received! Exiting." -> exit 130

Yes, awesome! Works for me. Merged.

joysn1980 claimed this task.EditedJan 17 2017, 4:18 PM

Is this task about doing this for all python scripts?

There 4 more in this directory

br_add:#!/usr/bin/python
generic_gui_message:#!/usr/bin/python
striphtml:#!/usr/bin/python
tb_updater_gui:#!/usr/bin/python

Are all these standalone python scripts? Are all these needs to be modified to have same changes - ctrl+c ?

Right. (ctrl+c as well as ctrl+d)

A few more projects are involved, where this is TODO. Btw there are all our components where python is involved.

Some of them (perhaps sdwdate and onion-grater (Control Port Filter Proxy)) might already have proper signal handling.

joysn1980 added a comment.EditedJan 18 2017, 9:06 AM

br_add - nothing to add
striphtml nothing to add
generic_gui_message - done
tb_updater_gui - done

https://github.com/joysn/msgcollector/tree/master/usr/lib/msgcollector

tested using
./generic_gui_message "info" "hi" "hi" 1 2
./tb_updater_gui "info" "hi" 1 2 "hi" "hi" "hi"

This is all for msgcollector

From sdwdate-gui

https://github.com/joysn/sdwdate-gui
/usr/bin/sdwdate-gui - changes made
tested using - ./sdwdate-gui
/usr/lib/sdwdate-gui/show_message - no change required
tested using ./show_message "hi" "1" "3"

From guimessages - 1 change required and it done
https://github.com/joysn/python-guimessages

/usr/share/pyshared/guimessages/guimessage.py - changes done
/usr/share/pyshared/guimessages/translations.py - no change required

From anon-shared-helper-scripts - no change required
/usr/lib/anon-shared-helper-scripts/tor_circuit_established_check.py - no change required
/usr/lib/anon-shared-helper-scripts/tor_circuit_established_check.py - no change required
/usr/lib/anon-shared-helper-scripts/tor_circuit_established_check.py - no change required

joysn1980 added a comment.EditedJan 18 2017, 9:46 AM

So we are done with

msgcollector - to be merged
sdwdate-gui - to be merged
python-guimessages - to be merged

No changes required for

sdwdate
tor-controlport-filter
anon-shared-helper-scripts

Let me know if there any places which still requires the changes, I can make them

joysn1980 added a comment.EditedJan 18 2017, 10:26 AM

whonix-setup-wizard, 1 change required, to be merged

This was left I guess
https://github.com/joysn/whonix-setup-wizard

/usr/bin/whonix-setup-wizard - changes done

Summary

msgcollector - to be merged
sdwdate-gui - to be merged
python-guimessages - to be merged
whonix-setup-wizard - to be merged

Patrick closed this task as Resolved.Jan 18 2017, 12:41 PM

Excellent! All merged.