Skip to content

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

Merged
OlegHahm merged 1 commit intoRIOT-OS:masterfrom
daniel-k:fix/openocd-closing-ctrl-c
Aug 31, 2015
Merged

debugging: fix openocd closing when pressing Ctrl+C in GDB, fix #3427#3619
OlegHahm merged 1 commit intoRIOT-OS:masterfrom
daniel-k:fix/openocd-closing-ctrl-c

Conversation

@daniel-k
Copy link
Copy Markdown
Member

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.

@OlegHahm 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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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 [email protected]:

@@ -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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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: $$"

@daniel-k
Copy link
Copy Markdown
Member Author

@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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

use mktemp instead.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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
Copy link
Copy Markdown
Member

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

@daniel-k
Copy link
Copy Markdown
Member Author

@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 :)

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

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
Copy link
Copy Markdown
Member Author

@thomaseichinger done

@thomaseichinger
Copy link
Copy Markdown
Member

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

@daniel-k daniel-k force-pushed the fix/openocd-closing-ctrl-c branch from 774222d to 8258707 Compare August 27, 2015 12:58
@daniel-k 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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

[ -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?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

sorry, I meant -z

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Anyway, where is $SETSID supposed to be set?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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" [email protected] 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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

@OlegHahm
Copy link
Copy Markdown
Member

Wanna squash again?

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

@OlegHahm
Squashed. Did you test again?

@OlegHahm
Copy link
Copy Markdown
Member

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

@daniel-k
Copy link
Copy Markdown
Member Author

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

@jnohlgard
Copy link
Copy Markdown
Member

jnohlgard commented Aug 31, 2015 via email

@OlegHahm OlegHahm assigned miri64 and unassigned thomaseichinger Aug 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

openocd 0.9: OpenOCD closes when pressing CTRL+C in GDB

5 participants