flux-framework / flux-core

core services for the Flux resource management framework
GNU Lesser General Public License v3.0
168 stars 50 forks source link

libsubprocess: take reference on callbacks #6384

Closed chu11 closed 1 month ago

chu11 commented 1 month ago

Problem: There is always a chance a user may destroy a subprocess object when a callback is called (i.e. output callback, state change callback, etc.). This can lead to problems if libsubprocess believes the object is still valid when the callback returns. References are taken on the subprocess object when issuing some callbacks, but it is missing in other callbacks.

Take reference on all callbacks to ensure safety if the user choses the destroy the subprocess in the callback.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 88.23529% with 2 lines in your changes missing coverage. Please review.

Project coverage is 83.49%. Comparing base (b37f4fa) to head (b53e516). Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
src/common/libsubprocess/local.c 83.33% 1 Missing :warning:
src/common/libsubprocess/remote.c 90.90% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #6384 +/- ## ========================================== - Coverage 83.49% 83.49% -0.01% ========================================== Files 524 524 Lines 87668 87683 +15 ========================================== + Hits 73200 73211 +11 - Misses 14468 14472 +4 ``` | [Files with missing lines](https://app.codecov.io/gh/flux-framework/flux-core/pull/6384?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework) | Coverage Δ | | |---|---|---| | [src/common/libsubprocess/local.c](https://app.codecov.io/gh/flux-framework/flux-core/pull/6384?src=pr&el=tree&filepath=src%2Fcommon%2Flibsubprocess%2Flocal.c&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-c3JjL2NvbW1vbi9saWJzdWJwcm9jZXNzL2xvY2FsLmM=) | `84.13% <83.33%> (+0.47%)` | :arrow_up: | | [src/common/libsubprocess/remote.c](https://app.codecov.io/gh/flux-framework/flux-core/pull/6384?src=pr&el=tree&filepath=src%2Fcommon%2Flibsubprocess%2Fremote.c&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework#diff-c3JjL2NvbW1vbi9saWJzdWJwcm9jZXNzL3JlbW90ZS5j) | `79.77% <90.90%> (+0.69%)` | :arrow_up: | ... and [13 files with indirect coverage changes](https://app.codecov.io/gh/flux-framework/flux-core/pull/6384/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flux-framework)