debugging: fix openocd closing when pressing Ctrl+C in GDB, fix #3427 #3619

daniel-k · 2015-08-12T16:17:10Z

Hopefully fixes #3427.

Please test this on as many system as possible, especially on Mac OS. The problem seemed to be, that openocd stays in the same process group when forking it (openocd .... &). Now in GDB when you press Ctrl+c the SIGINT signal will be sent to the parent shell, that redirects it to GDB as well as to openocd.

Solution: detach openocd from process group.

The script also used the wrong environment variable to grab the openocd PID accroding to this.

jnohlgard · 2015-08-12T17:17:32Z

dist/tools/openocd/openocd.sh

@@ -202,7 +202,7 @@ do_debug() {
            -c 'halt' \
            -l /dev/null" &
    # save PID for terminating the server afterwards
-    OCD_PID=$?
+    OCD_PID=$(($!+2))


the +2 feels kind of arbitrary. Is it guaranteed to be the same every time on every system?

This bugs me too, but I didn't find a better solution using standard POSIX tools. That's why this needs to be tested on many systems. Any suggestions?

Am 12. August 2015 19:17:40 MESZ, schrieb Joakim Gebart notifications@github.com:

@@ -202,7 +202,7 @@ do_debug() {
-c 'halt'
-l /dev/null" &
# save PID for terminating the server afterwards

OCD_PID=$?

OCD_PID=$(($!+2))

the +2 feels kind of arbitrary. Is it guaranteed to be the same every
time on every system?

Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/3619/files#r36886983

an idea:
use the $$ variable in combination with some variation of sh -c "echo \$\$; xxx" & and some output redirection

Test with: setsid sh -c "echo child: \$\$; sleep 2" &; echo "setsid: $\!"; echo "shell: $$"

jnohlgard · 2015-08-12T17:25:07Z

related info: http://blog.n01se.net/blog-n01se-net-p-145.html

daniel-k · 2015-08-12T21:42:49Z

@gebart
Nice idea, even though I've found a slightly different approach now. I didn't dare to mess with output redirection but save the openocd pid in a temporary file.

jnohlgard · 2015-08-13T07:08:22Z

dist/tools/openocd/openocd.sh

@@ -191,8 +191,10 @@ do_debug() {
    test_elffile
    test_ports
    test_tui
+    # temporary file that saves OpenOCD pid
+    tmp=openocd_`date +%s`.pid


use mktemp instead.

here's an example of using mktemp from another script I wrote:

TMPFILE= trap '[ -n "${TMPFILE}"] && rm -f "${TMPFILE}"' EXIT TMPFILE=$(mktemp "${TARGET_DIR}/my_temp_file.XXXXXXXXXX") wget "${SOURCE_URI}" -O "${TMPFILE}" && ( chmod a+r "${TMPFILE}"; mv -b -f "${TMPFILE}" "${TARGET_DIR}/the_real_target.zip" ) trap - EXIT

thomaseichinger · 2015-08-19T15:17:56Z

Changes look reasonable, but please make them conditional to Linux. Didn't hear about the Ctrl+C problem on these platforms anyway.

daniel-k · 2015-08-26T13:23:00Z

@thomaseichinger Could you please test again on Mac and FreeBSD? Hopefully I used mktemp the right way and trap also behaves the same as on Linux :)

thomaseichinger · 2015-08-27T12:08:58Z

Seems I was testing against the wrong OpenOCD version. This problem does exist on OS X too. To get this in quickly, could you change to test for presence of setsid? Let's try to find a general OS X workaround later.

There does exist a setsid workaround tool for OS X which does what it should. (I created a Formula to install it with homebrew)

daniel-k · 2015-08-27T12:36:00Z

@thomaseichinger done

thomaseichinger · 2015-08-27T12:56:50Z

Works for me. Could an other Linux user verify? But I think you can proceed with squashing.

jnohlgard · 2015-08-27T13:08:05Z

dist/tools/openocd/openocd.sh

@@ -191,8 +191,16 @@ do_debug() {
    test_elffile
    test_ports
    test_tui
+    # setsid is needed so that Ctrl+C in GDB doesn't kill OpenOCD
+    [ -n "$(which setsid)" ] && SETSID=setsid


Actually, it makes more sense to check whether the SETSID variable is null. (sorry for commenting in the commit before)

[ -z "${SETSID}" ] && SETSID=$(which setsid)

edit: fixed mixup between -n, -z

[ -n "${SETSID}" ] && SETSID=$(which setsid)

I don't get this. If $SETSID is not empty, then set it to the output of which setsid? Why should $SETSID be set in the first place?

sorry, I meant -z

Anyway, where is $SETSID supposed to be set?

The purpose was to allow overriding it from the environment in the case
where you have the osx workaround installed somewhere outside of the normal
path
On Aug 27, 2015 3:23 PM, "Daniel Krebs" notifications@github.com wrote:

In dist/tools/openocd/openocd.sh
#3619 (comment):

@@ -191,8 +191,16 @@ do_debug() {
test_elffile
test_ports
test_tui

setsid is needed so that Ctrl+C in GDB doesn't kill OpenOCD

[ -n "$(which setsid)" ] && SETSID=setsid

Anyway, where is $SETSID supposed to be set?

—
Reply to this email directly or view it on GitHub
https://github.com/RIOT-OS/RIOT/pull/3619/files#r38093810.

I see. And I realized that the checking isn't needed in my solution, because SETSID="$(which setsid)" already does it.

OlegHahm · 2015-08-29T10:42:24Z

Wanna squash again?

…OS#3427

daniel-k · 2015-08-29T17:17:55Z

@OlegHahm
Squashed. Did you test again?

OlegHahm · 2015-08-30T20:51:40Z

I never had this problem, but doesn't seem to break anything. @gebart, @authmillenon, can you verify?

daniel-k · 2015-08-31T09:36:39Z

It seems to be related to recent OpenOCD versions, maybe >= 0.9.0

jnohlgard · 2015-08-31T12:34:02Z

I won't be able to verify until the end of next week at the earliest.

miri64 · 2015-08-31T19:59:56Z

Works for me:

[martine@beutlin RIOT]<3 openocd --version
Open On-Chip Debugger 0.9.0-dev-00358-gd3c2679 (2015-04-03-14:36)
Licensed under GNU GPL v2
For bug reports, read
    http://openocd.sourceforge.net/doc/doxygen/bugs.html
[martine@beutlin RIOT]<3 uname -a
Linux beutlin 3.13.0-24-generic #47-Ubuntu SMP Fri May 2 23:30:00 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux

If this is enough I give this PR an ACK. If someone else wants to test this, please do.

OlegHahm · 2015-08-31T21:44:07Z

Works for at least four maintainers on different platforms - should be good enough.

debugging: fix openocd closing when pressing Ctrl+C in GDB, fix #3427

OlegHahm assigned thomaseichinger Aug 12, 2015

OlegHahm added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: tools Area: Supplementary tools labels Aug 12, 2015

jnohlgard reviewed Aug 12, 2015
View reviewed changes

jnohlgard reviewed Aug 13, 2015
View reviewed changes

daniel-k force-pushed the fix/openocd-closing-ctrl-c branch from 3545021 to ee19baa Compare August 27, 2015 11:21

daniel-k force-pushed the fix/openocd-closing-ctrl-c branch from 774222d to 8258707 Compare August 27, 2015 12:58

daniel-k added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 27, 2015

jnohlgard reviewed Aug 27, 2015
View reviewed changes

daniel-k mentioned this pull request Aug 27, 2015

Simple duty cycling 802.15.4 MAC protocol #3730

Closed

debugging: fix openocd closing when pressing Ctrl+C in GDB, fix RIOT-…

78b98b3

…OS#3427

daniel-k force-pushed the fix/openocd-closing-ctrl-c branch from 58689ed to 78b98b3 Compare August 29, 2015 17:17

OlegHahm assigned miri64 and unassigned thomaseichinger Aug 31, 2015

OlegHahm added a commit that referenced this pull request Aug 31, 2015

Merge pull request #3619 from daniel-k/fix/openocd-closing-ctrl-c

6b41a5f

debugging: fix openocd closing when pressing Ctrl+C in GDB, fix #3427

OlegHahm merged commit 6b41a5f into RIOT-OS:master Aug 31, 2015

daniel-k deleted the fix/openocd-closing-ctrl-c branch September 16, 2015 13:25

salkinium mentioned this pull request Aug 7, 2018

Move OpenOCD and GDB tools into parent build module modm-io/modm#56

Merged

2 tasks

debugging: fix openocd closing when pressing Ctrl+C in GDB, fix #3427 #3619

debugging: fix openocd closing when pressing Ctrl+C in GDB, fix #3427 #3619

Uh oh!

Conversation

daniel-k commented Aug 12, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnohlgard commented Aug 12, 2015

Uh oh!

daniel-k commented Aug 12, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thomaseichinger commented Aug 19, 2015

Uh oh!

daniel-k commented Aug 26, 2015

Uh oh!

thomaseichinger commented Aug 27, 2015

Uh oh!

daniel-k commented Aug 27, 2015

Uh oh!

thomaseichinger commented Aug 27, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

setsid is needed so that Ctrl+C in GDB doesn't kill OpenOCD

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OlegHahm commented Aug 29, 2015

Uh oh!

daniel-k commented Aug 29, 2015

Uh oh!

OlegHahm commented Aug 30, 2015

Uh oh!

daniel-k commented Aug 31, 2015

Uh oh!

jnohlgard commented Aug 31, 2015 via email

Uh oh!

miri64 commented Aug 31, 2015

Uh oh!

OlegHahm commented Aug 31, 2015

Uh oh!

Uh oh!