Skip to content

pyterm: send SIGINT to the subprocess' child #21309

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

Jump to bottom
Merged
merged 1 commit into from
Mar 20, 2025
Merged

pyterm: send SIGINT to the subprocess' child #21309

merged 1 commit into from
Mar 20, 2025

Conversation

OlegHahm
Copy link
Member

Contribution description

Upon terminating pyterm any potentially spawned process (such as RIOT native) should be terminated as well. This could be done via sending a SIGINT to these processes.

Testing procedure

Run any arbitrary RIOT application for RIOT native via make term and terminate pyterm (via /exit or ^C).

Check if all RIOT native processes have been terminated.

Issues/PRs references

Fixes #21307

@github-actions github-actions bot added the Area: tools Area: Supplementary tools label Mar 19, 2025
@OlegHahm OlegHahm added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Platform: native Platform: This PR/issue effects the native platform labels Mar 19, 2025
@OlegHahm OlegHahm requested a review from benpicco March 19, 2025 20:14
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 20, 2025
@riot-ci
Copy link

riot-ci commented Mar 20, 2025 •
edited
Loading

Murdock results

✔️ PASSED

0de3b8a pyterm: send SIGINT to the subprocess' child

Success Failures Total Runtime
1 0 1 01m:20s

Artifacts

@benpicco
Copy link
Contributor

Thank you, this fixes the issue of lingering native instances when pressing ctrl+c

They still linger around when exiting pyterm with ctrl+d though:

benpicco@carbonara ..amples/networking/gnrc/gnrc_networking % make term                                                                              
/home/benpicco/dev/RIOT/dist/tools/pyterm/pyterm -ps /home/benpicco/dev/RIOT/examples/networking/gnrc/gnrc_networking/bin/native/gnrc_networking.elf --process-args tap0 
Twisted not available, please install it if you want to use pyterm's JSON capabilities
Welcome to pyterm!
Type '/exit' to exit.
2025-03-20 01:44:51,281 # RIOT native interrupts/signals initialized.
2025-03-20 01:44:51,281 # RIOT native board initialized.
2025-03-20 01:44:51,281 # RIOT native hardware initialization complete.
2025-03-20 01:44:51,281 # 
2025-03-20 01:44:51,282 # main(): This is RIOT! (Version: 2025.04-devel-257-g61029-dec_as_hex)
2025-03-20 01:44:51,282 # RIOT network stack example application
2025-03-20 01:44:51,282 # All up, running the shell now
> 2025-03-20 01:44:52,032 # Exiting Pyterm

benpicco@carbonara ..amples/networking/gnrc/gnrc_networking % make term
/home/benpicco/dev/RIOT/dist/tools/pyterm/pyterm -ps /home/benpicco/dev/RIOT/examples/networking/gnrc/gnrc_networking/bin/native/gnrc_networking.elf --process-args tap0 
Twisted not available, please install it if you want to use pyterm's JSON capabilities
Welcome to pyterm!
Type '/exit' to exit.
2025-03-20 01:47:42,444 # RIOT native interrupts/signals initialized.
2025-03-20 01:47:42,445 # RIOT native board initialized.
2025-03-20 01:47:42,446 # RIOT native hardware initialization complete.
2025-03-20 01:47:42,446 # 
2025-03-20 01:47:42,448 # /home/benpicco/dev/RIOT/examples/networking/gnrc/gnrc_networking/bin/native/gnrc_networking.elf: ioctl TUNSETIFF: Device or resource busy
2025-03-20 01:47:42,450 # /home/benpicco/dev/RIOT/examples/networking/gnrc/gnrc_networking/bin/native/gnrc_networking.elf: probably the tap interface (tap0) does not exist or is already in use

benpicco
benpicco approved these changes Mar 20, 2025
View reviewed changes
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Works reliable for ctrl+c though

@OlegHahm
Copy link
Member Author

They still linger around when exiting pyterm with ctrl+d though:

Strange. I never use this but there is a handler in pyterm for that. But apparently it stopped working. Will take a look into it.

@OlegHahm
Copy link
Member Author

OlegHahm commented Mar 20, 2025 •
edited
Loading

Ah, no, it is still working - and for me the problem does not exist. The process is terminated on ^C and ^D. Could you check if the function in pyterm do_EOF() gets properly executed on your machine?

@benpicco
Copy link
Contributor

Yes I get that

2025-03-20 14:40:59,343 # Received EOF
2025-03-20 14:40:59,343 # Exiting Pyterm

but the native process keeps lingering on

With ctrl-c I get

2025-03-20 14:42:01,152 # native: exiting
2025-03-20 14:42:01,152 # 
2025-03-20 14:42:01,153 # native: exiting
2025-03-20 14:42:01,154 # Exiting Pyterm

@OlegHahm OlegHahm changed the title pyterm: send SIGINT to the subprocess\' child pyterm: send SIGINT to the subprocess' child Mar 20, 2025
@OlegHahm
Copy link
Member Author

Strange. The same code is run but the behavior is different - at least for you. On my computer the output looks similar to the one you have posted but the processes are gone in any case afterwards.

@OlegHahm
Copy link
Member Author

I tested in a OpenSuSE VM: the behavior there is that ^C terminates the processes immediately, for ^D or via /exit the processes keep lingering for some time but vanish after some seconds.

@github-actions github-actions bot removed the Platform: native Platform: This PR/issue effects the native platform label Mar 20, 2025
@OlegHahm
Copy link
Member Author

Can you give it another try? Should now work with ^D and /exit as well.

@OlegHahm OlegHahm added the Platform: native Platform: This PR/issue effects the native platform label Mar 20, 2025
@benpicco
Copy link
Contributor

benpicco commented Mar 20, 2025 •
edited
Loading

Yes much better - now it always terminates properly 😃
Please squash!

What system are you on where it's already doing that before? I'm on Ubuntu 24.04 with Python 3.12.3

@OlegHahm
Copy link
Member Author

Arch Linux with Python 3.13.2.

@OlegHahm OlegHahm force-pushed the pr/pyterm/sigint_at_exit branch from 5db1d92 to 0de3b8a Compare March 20, 2025 22:12
@github-actions github-actions bot removed the Platform: native Platform: This PR/issue effects the native platform label Mar 20, 2025
@OlegHahm OlegHahm enabled auto-merge March 20, 2025 22:13
@OlegHahm OlegHahm added this pull request to the merge queue Mar 20, 2025
Merged via the queue into RIOT-OS:master with commit 3312892 Mar 20, 2025
25 checks passed
@OlegHahm OlegHahm deleted the pr/pyterm/sigint_at_exit branch March 20, 2025 22:22
mguetschow
mguetschow reviewed Mar 21, 2025
View reviewed changes
dist/tools/pyterm/pyterm
@@ -38,8 +38,10 @@ import argparse
import re
import codecs
import platform
import psutil
Copy link
Contributor

Choose a reason for hiding this comment

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

@OlegHahm This introduced a new dependency that may not be installed by default. Is there an alternative to it or should we add psutil to the list of dependencies stated on https://doc.riot-os.org/getting-started.html?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this has broken pyterm for me. I now get:

josh@dragonkite:ds18$ make BOARD=stm32f429i-disc1 term
/home/josh/riot/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200" -ln "/tmp/pyterm-josh" -rn "2025-04-09_22.09.13-tests_ds18-stm32f429i-disc1"  
Traceback (most recent call last):
  File "/home/josh/riot/dist/tools/pyterm/pyterm", line 41, in <module>
    import psutil
ModuleNotFoundError: No module named 'psutil'
make: *** [/home/josh/riot/tests/drivers/ds18/../../../Makefile.include:873: term] Error 1

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, you will need to install the Python psutil module. I didn't realize that psutil is not a standard Python module. 😔 If anyone would find the time to re-implement this feature without an external modul that would be wonderful.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is sufficient to add it to the list of pkgs to install in the getting started guide. See #21399.

@mguetschow mguetschow added this to the Release 2025.04 milestone Apr 8, 2025
@Enoch247 Enoch247 mentioned this pull request Apr 12, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Enoch247 Enoch247 Enoch247 left review comments

@mguetschow mguetschow mguetschow left review comments

@benpicco benpicco benpicco approved these changes

Assignees

@benpicco benpicco

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
Milestone
Release 2025.04
Development

Successfully merging this pull request may close these issues.

native: Exiting pyterm does not terminate the native process any more
5 participants
@OlegHahm @riot-ci @benpicco @Enoch247 @mguetschow