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

Suppress all exceptions during DeleteOnExitPathHook #1409

Merged
merged 1 commit into from
Aug 19, 2019

Conversation

lbergelson
Copy link
Member

@lbergelson lbergelson commented Aug 14, 2019

Description

  • Currently we were not catching some commonly thrown exceptions like RuntimeIOException while deleting files at shutdown.
    This results in confusing error messages at program exit which can mask the actual cause of failures.
    Now we catch all exceptions and output a log message at debug level in case anyone is interested in seeing the failures.

* Currently we were not catching some commonly thrown exceptions like RuntimeIOException while deleting files at shutdown.
  This results in confusing error messages at program exit which can mask the actual cause of failures.
  Now we catch all exceptions and output a log message at debug level in case anyone is interested in seeing the failures.
@codecov-io
Copy link

codecov-io commented Aug 14, 2019

Codecov Report

Merging #1409 into master will increase coverage by 0.002%.
The diff coverage is 100%.

@@               Coverage Diff               @@
##              master     #1409       +/-   ##
===============================================
+ Coverage     68.101%   68.102%   +0.002%     
  Complexity      8373      8373               
===============================================
  Files            573       573               
  Lines          33963     33965        +2     
  Branches        5668      5668               
===============================================
+ Hits           23129     23131        +2     
+ Misses          8645      8644        -1     
- Partials        2189      2190        +1
Impacted Files Coverage Δ Complexity Δ
...htsjdk/samtools/util/nio/DeleteOnExitPathHook.java 80.952% <100%> (-8.521%) 3 <1> (-1)
src/main/java/htsjdk/samtools/util/Log.java 54.321% <0%> (+2.469%) 21% <0%> (+1%) ⬆️

Copy link
Collaborator

@cmnbroad cmnbroad left a comment

Choose a reason for hiding this comment

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

@lbergelson Seems like a good idea in general, but do you know if logging during shutdown works correctly ?

@lbergelson
Copy link
Member Author

I don't know why it wouldn't. It's basically just writing to system.err so unless you're 100% out of memory it doesn't seem like it would be problematic. I googled for restrictions on shutdown hooks and the only thing that's definitely not allowed is registering a new shutdown hook. I'm definitely not an expert on it though, so I might be wrong...

@lbergelson lbergelson merged commit a798e4e into master Aug 19, 2019
@lbergelson lbergelson deleted the lb_suppress_shutdown_hook_exceptions branch August 19, 2019 18:08
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.

3 participants