Skip to content
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.

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

close and reset Python subprocess state on clear #1016

Merged
merged 1 commit into from Mar 29, 2022

Conversation

mtmiller
Copy link
Collaborator

@mtmiller mtmiller commented Apr 1, 2020

Move popen2 reset logic to a new helper function, and call it using a
persistent onCleanup object.

Fixes #1015.

Move popen2 reset logic to a new helper function, and call it using a
persistent onCleanup object.

Fixes gnu-octave#1015.
t = fclose(fout); fout = [];
A = A && (t == 0);
end
cleanup = [];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This assignment triggers a call to the new reset helper function

fin = [];
fout = [];
pid = [];
A = true;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately I can't figure out a way to both use onCleanup and capture the return value

%% @var{A} is the resulting object, which might be an error code.
%% @end deftypefun

function A = python_ipc_popen2_reset (fin, fout, pid)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe no point in returning any status here now?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I agree. I'm merging this to a local branch so I can try it on CI as well as try removing these return vlaues.

@@ -0,0 +1,51 @@
%% Copyright (C) 2020 Mike Miller
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This file was copied from python_ipc_popen2.m, do you think me should propagate the original Copyright here?

@cbm755 cbm755 changed the base branch from master to popen2-clear-all March 29, 2022 19:05
@cbm755 cbm755 merged commit 21b8b4c into gnu-octave:popen2-clear-all Mar 29, 2022
@mtmiller mtmiller deleted the popen2-clear-all branch April 11, 2022 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

close Python subprocess on clear all or clear functions
2 participants