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 file descriptors after process exit #5983

Merged
merged 1 commit into from
Sep 16, 2021

Conversation

mih
Copy link
Member

@mih mih commented Sep 15, 2021

Even a datalad --version leave open file descriptors around. This
change closes all FDs after a process has exited and any
protocol-based post-processing has completed.

I am not at all confident that this should be done unconditionally,
though.

Run PYTHONDEVMODE=1 datalad --version to see the pre/post effect.

Closes #5965

Even a `datalad --version` leave open file descriptors around. This
change closes all FDs after a process has exited and any
protocol-based post-processing has completed.

I am not at all confident that this should be done unconditionally,
though.
@yarikoptic yarikoptic added the semver-patch Increment the patch version when merged label Sep 15, 2021
@yarikoptic
Copy link
Member

oh, is that the fix for #5965 ? !

@codecov
Copy link

codecov bot commented Sep 15, 2021

Codecov Report

Merging #5983 (7346365) into maint (1e0f732) will decrease coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##            maint    #5983      +/-   ##
==========================================
- Coverage   90.32%   90.23%   -0.09%     
==========================================
  Files         308      308              
  Lines       42111    42114       +3     
==========================================
- Hits        38035    38001      -34     
- Misses       4076     4113      +37     
Impacted Files Coverage Δ
datalad/nonasyncrunner.py 100.00% <100.00%> (ø)
datalad/core/local/tests/test_results.py 92.85% <0.00%> (-7.15%) ⬇️
datalad/core/local/tests/test_diff.py 97.23% <0.00%> (-2.31%) ⬇️
datalad/core/distributed/clone.py 90.82% <0.00%> (-1.61%) ⬇️
datalad/core/local/tests/test_status.py 97.32% <0.00%> (-0.90%) ⬇️
datalad/interface/utils.py 94.58% <0.00%> (-0.73%) ⬇️
datalad/tests/utils.py 88.90% <0.00%> (-0.65%) ⬇️
datalad/core/distributed/tests/test_push.py 98.30% <0.00%> (-0.64%) ⬇️
datalad/core/local/tests/test_save.py 97.49% <0.00%> (-0.54%) ⬇️
datalad/core/distributed/tests/test_clone.py 97.05% <0.00%> (-0.39%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e0f732...7346365. Read the comment docs.

@mih
Copy link
Member Author

mih commented Sep 15, 2021

That would fit the description, yes.

@yarikoptic
Copy link
Member

confirming -- it does! Thank you again! will add close statement
(git)lena:~datalad/datalad-maint[maint]git
$> datalad --version
datalad 0.15.0+2.g1e0f732c1
(dev3) 1 15305.....................................:Wed 15 Sep 2021 11:45:05 AM EDT:.
(git)lena:~datalad/datalad-maint[maint]git
$> DATALAD_LOG_LEVEL=1 python -Werror -c 'import datalad.version as m;print(m.__version__)'
[Level 5] Instantiating ssh manager 
[Level 5] Done importing main __init__ 
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/yoh/proj/datalad/datalad-maint/datalad/version.py", line 17, in <module>
    warnings.warn(
DeprecationWarning: datalad.version module will be removed in 0.16. Please use datalad.__version__ (no other __*_version__ variables are to be provided).
[Level 5] Exiting 
Exception ignored in: <_io.FileIO name=3 mode='rb' closefd=True>
ResourceWarning: unclosed file <_io.FileIO name=3 mode='rb' closefd=True>
Exception ignored in: <_io.FileIO name=5 mode='rb' closefd=True>
ResourceWarning: unclosed file <_io.FileIO name=5 mode='rb' closefd=True>
(dev3) 1 15306 ->1.....................................:Wed 15 Sep 2021 11:45:26 AM EDT:.
(git)lena:~datalad/datalad-maint[maint]git
$> git fetch gh-mih 
From https://github.com/hanke/datalad
 * [new branch]          bf-closefd   -> gh-mih/bf-closefd
   ff6f7eacc..5178ae629  maint        -> gh-mih/maint
 * [new branch]          nf-gogs      -> gh-mih/nf-gogs
 * [new branch]          rf-pbsrunner -> gh-mih/rf-pbsrunner
(dev3) 1 15307.....................................:Wed 15 Sep 2021 11:45:33 AM EDT:.
(git)lena:~datalad/datalad-maint[maint]git
$> git co bf-closefd 
Branch 'bf-closefd' set up to track remote branch 'bf-closefd' from 'gh-mih'.
Switched to a new branch 'bf-closefd'
(dev3) 1 15308.....................................:Wed 15 Sep 2021 11:45:40 AM EDT:.
(git)lena:~datalad/datalad-maint[bf-closefd]git
$> DATALAD_LOG_LEVEL=1 python -Werror -c 'import datalad.version as m;print(m.__version__)'
[Level 5] Instantiating ssh manager 
[Level 5] Done importing main __init__ 
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/yoh/proj/datalad/datalad-maint/datalad/version.py", line 17, in <module>
    warnings.warn(
DeprecationWarning: datalad.version module will be removed in 0.16. Please use datalad.__version__ (no other __*_version__ variables are to be provided).
[Level 5] Exiting 
(dev3) 1 15309 ->1.....................................:Wed 15 Sep 2021 11:45:42 AM EDT:.
(git)lena:~datalad/datalad-maint[bf-closefd]git
$> DATALAD_LOG_LEVEL=1 python -Werror -c 'import datalad as m;print(m.__version__)'        
[Level 5] Instantiating ssh manager 
[Level 5] Done importing main __init__ 
0.15.0+3.g7346365ce
[Level 5] Exiting 

Copy link
Member

@yarikoptic yarikoptic left a comment

Choose a reason for hiding this comment

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

CI is green, I am happy ;) Thank you @mih ! Let's wait for verdict by @christian-monch

@yarikoptic yarikoptic added this to the 0.15.1 milestone Sep 15, 2021
@christian-monch
Copy link
Contributor

Looks good. I think the files can be closed unconditionally in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-patch Increment the patch version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

new ignored warnings: ResourceWarning: unclosed file upon import of datalad
3 participants