[#722] fix segfault and hung threads on KeyboardIinterrupt during parallel get #728

d-w-moore · 2025-05-19T17:07:54Z

Wherein we close down threads in an orderly way, so that things don't leave things to be disposed in the wrong order for the ever persnickety SSL shutdown logic.

Experiments show that SIGTERM actually does induce the Python interpreter to shut down non-daemonic threads, so installing a signal handler for that may not be necessary in the end.

d-w-moore · 2025-05-19T17:10:10Z

After a bit of manual testing, will attempt to make a proper test for SIGINT and SIGTERM to ensure things are left in an ok state.

d-w-moore · 2025-06-05T18:12:49Z

A GUI for example that maintains background asynch parallel transfers using PRC could trap and guard against Ctrl-C thusly:

from irods.parallel import abort_asynchronous_transfers
signal(SIGINT, lambda *_:exit(0 if abort_asynchronous_transfers() else 0))

Update: abort_asynchronous_transfers has transformed. It is now abort_parallel_transfers and may now be used to abort the current (just interrupted) synchronous transfer as well as all pending background ones. See the README updates in this pull request.

alanking

Seems reasonable. Just a couple of things in the test

irods/test/modules/test_signal_handling_in_multithread_get.py

korydraughn · 2025-07-08T18:27:55Z

Looks like we have a conflict.

Seems this PR is close to completion?

alanking · 2025-12-05T21:14:07Z

Just checking to see if this PR is still being considered for 3.3.0.

d-w-moore · 2025-12-08T08:15:20Z

Just checking to see if this PR is still being considered for 3.3.0.

Will check its currency to see whether the segfault is still a concern. If so, then I think we can consider it for this release.

irods/test/modules/test_signal_handling_in_multithread_get.py

irods/parallel.py

README.md

irods/manager/data_object_manager.py

irods/parallel.py

korydraughn · 2026-01-06T15:21:06Z

What's the status of this PR?

d-w-moore · 2026-01-07T15:27:17Z

What's the status of this PR?

I believe it's almost ready. I want to look over it once more.

alanking

Awaiting signal that this is ready

d-w-moore · 2026-01-08T20:02:23Z

Awaiting signal that this is ready

I've added a test (first draft, will run soon) that interrupts a put . We weren't testing that previously.

d-w-moore · 2026-01-15T20:07:51Z

Now open to comment ... once more. These changes are final in my mind, as far as the significant changes to library functionality.

d-w-moore · 2026-01-15T20:08:59Z

I can squash the last 6 commits or so, if that helps reviewers.

korydraughn · 2026-01-15T20:12:56Z

Yes please.

irods/parallel.py

irods/test/modules/test_signal_handling_in_multithread_put.py

irods/test/modules/test_signal_handling_in_multithread_get.py

alanking · 2026-01-15T21:01:56Z

Also, take a peek at the 2 failing checks to see if there's anything actionable.

d-w-moore · 2026-01-15T21:24:18Z

Sorry about the delay. (I see some reviews are already made.) I have just submitted a squash - but no actual changes of any kind since the last note I posted above.

d-w-moore · 2026-01-15T21:55:53Z

Still a TODO: Introduce a DataTransferInterruptedException and raise the RuntimeError from it, in the case that we need to signal a put or get didn't complete. We could also, possibly, deprecate catching the RuntimeError in case at time of 4.0.0 release we want to make it a bare DataTransferInterruptedException. (Name up for suggestions, but that's what I've settled on.)

Going to have to make an issue for this one and handle it later, that is in 4.0.0, I think. Things are looking pretty complex so far, and I don't want to complicate this release with unforeseen and as yet too-difficult-to-test problems....

alanking · 2026-01-20T16:18:29Z

Awaiting resolution of open review comments.

d-w-moore · 2026-01-20T20:56:20Z

Awaiting resolution of open review comments.

Will take care of those later today. For now, putting in work done tho' new tests do not pass.

Multithread programming is evil.

… transfers We now provide utility for bringing down parallel PUTs and GETs in an orderly way. The segmentation faults could not be duplicated, although they are more likely in general when aborting a main process that has spawned daemon threads. Note that since 3.9 (the version we now support at a minimum), Python no longer uses daemon threads in support of concurrent.futures. use subtest. try to preserve latest synchronous parallel put/get for orderly shutdown in signal handler can now abort parallel transfers with SIGINT/^C or SIGTERM some debug still remains. [_722] update readme for signals and parallel put/get prevent auto_close satisfy static typing. revise README forward ref needed for mypy? patch test more informative error message when retcodes do not match delete unnecessary "import irods" Update README.md Co-authored-by: Alan King <[email protected]> add a finite timeout review comments comments regarding futures returning None test condition wait is ten minutes is the default, no need to specify in call catch was a no-op remove TODO's [_722] test a data put is sanely interruptable [squashed multiple commits] tighten up all the quit logic: finish put test debug(parallel) debug(put-test) behaves better if we add mgr to list sooner? experimental changes ACTIVE_PATH paths active make return values consisten from io_multipart_*() print debug on abort almost there? move statement where transfer_managers is updated rework abort_transfer fn slightly handle logic for prematurely shutdown executor [another_squash] tidy, fix, add put test add tools.py with shared functions. make doc string more thorough, for abort_parallel_transfers(). codacy, review ws update README on abort_parallel_transfers resolve and display causes of error as is best possible currently we just get out cleanly, and make sure the causation is preserved. Later (v4.0) we may introduce a TransferInterrupted or similar which can be more useful than RuntimeError for indicating what happened. (in place of the RuntimeError("xxx failed.") in irods/manager/data_object_manager.) whitespace gettest cond in handler alter get test for multiple abort M.O. (exit from hdlr vs catch exc) ensure multiple calls to quit() are not inefficient (but, in any case they are safe.) noprint by default abort_parallel_transfers should not increase reference counts. extra param dicttype remove README whitespace comment,filter fnsf generalize return code for tests dictionary_type recognized as a more general parameter, "transform" review comment (needed explanation for print) deambiguate f -> future with previous mention.

… attached to RuntimeError

korydraughn · 2026-01-21T16:06:50Z

README.md

+```python
+from irods.parallel import abort_parallel_transfers
+
+def handler(signal, _)


Suggested change

def handler(signal, _)

def handler(signal, _):

korydraughn · 2026-01-21T16:33:54Z

README.md

+
+def handler(signal, _)
+    abort_parallel_transfers()
+    os._exit(128 + signal)


From discussion - remove os._exit.

d-w-moore changed the title ~~[_722] fix segfault and hung threads on SIGINT during parallel get~~ [#722] fix segfault and hung threads on KeyboardIinterrupt during parallel get May 19, 2025

d-w-moore self-assigned this May 19, 2025

d-w-moore marked this pull request as draft May 19, 2025 17:09

alanking mentioned this pull request May 21, 2025

Segmentation fault and ssl errors after the client connection is cut by KeyboardInterrupt #722

Open

d-w-moore force-pushed the segfault_parallel_io_722.m branch from e9d96e2 to 7e9a09f Compare May 22, 2025 13:02

d-w-moore marked this pull request as ready for review June 5, 2025 17:34

d-w-moore force-pushed the segfault_parallel_io_722.m branch from abafff5 to fb36836 Compare June 6, 2025 13:48

alanking reviewed Jun 30, 2025

View reviewed changes

irods/test/modules/test_signal_handling_in_multithread_get.py Outdated Show resolved Hide resolved

irods/test/modules/test_signal_handling_in_multithread_get.py Outdated Show resolved Hide resolved

irods/test/modules/test_signal_handling_in_multithread_get.py Outdated Show resolved Hide resolved

d-w-moore force-pushed the segfault_parallel_io_722.m branch from fb36836 to 481952c Compare December 12, 2025 15:25

alanking reviewed Dec 19, 2025

View reviewed changes

irods/test/modules/test_signal_handling_in_multithread_get.py Outdated Show resolved Hide resolved

alanking reviewed Jan 2, 2026

View reviewed changes

irods/parallel.py Outdated Show resolved Hide resolved

irods/parallel.py Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

irods/manager/data_object_manager.py Outdated Show resolved Hide resolved

irods/parallel.py Outdated Show resolved Hide resolved

alanking reviewed Jan 8, 2026

View reviewed changes

alanking reviewed Jan 15, 2026

View reviewed changes

d-w-moore force-pushed the segfault_parallel_io_722.m branch from 4b0458e to 14037f9 Compare January 15, 2026 21:23

d-w-moore force-pushed the segfault_parallel_io_722.m branch from e3b5599 to 2700f86 Compare January 21, 2026 10:29

for parallel put/get, iRODSExceptions should go through directly, not…

3548b0d

… attached to RuntimeError

korydraughn reviewed Jan 21, 2026

View reviewed changes

[#722] fix segfault and hung threads on KeyboardIinterrupt during parallel get #728

Are you sure you want to change the base?

[#722] fix segfault and hung threads on KeyboardIinterrupt during parallel get #728

Uh oh!

Conversation

d-w-moore commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

d-w-moore commented May 19, 2025

Uh oh!

d-w-moore commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alanking left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

korydraughn commented Jul 8, 2025

Uh oh!

alanking commented Dec 5, 2025

Uh oh!

d-w-moore commented Dec 8, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

korydraughn commented Jan 6, 2026

Uh oh!

d-w-moore commented Jan 7, 2026

Uh oh!

alanking left a comment

Choose a reason for hiding this comment

Uh oh!

d-w-moore commented Jan 8, 2026

Uh oh!

d-w-moore commented Jan 15, 2026

Uh oh!

d-w-moore commented Jan 15, 2026

Uh oh!

korydraughn commented Jan 15, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alanking commented Jan 15, 2026

Uh oh!

d-w-moore commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

d-w-moore commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alanking commented Jan 20, 2026

Uh oh!

d-w-moore commented Jan 20, 2026

Uh oh!

korydraughn Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

korydraughn Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

d-w-moore commented May 19, 2025 •

edited

Loading

d-w-moore commented Jun 5, 2025 •

edited

Loading

d-w-moore commented Jan 15, 2026 •

edited

Loading

d-w-moore commented Jan 15, 2026 •

edited

Loading