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

Bubble Throwables when executing Runnables #112

Merged
merged 1 commit into from
Nov 13, 2017

Conversation

jeremysears
Copy link
Contributor

This change updates calls to ExecutorService.submit (mostly deamons), to
call ExecutorService.execute instead. The submit variant returns a
Future which will contain any errors that are thrown from the Runnable.
However, nothing was checking these throwables for errors. If an error
happens, for example an OutOfMemoryError or RuntimeException thrown from
a custom AWSCredentialsProvider, the deamon thread will die without ever
logging any error information. Changing this to use execute allows the
Thread's UncaughtExceptionHandler or the default
UncaughtExceptionHandler to properly handle the failure. At a minimum
this allows the error to be logged. Some clients may wish to respond to
an OutOfMemoryError by taking a more severe action, such as restarting
the service. Given that failure of these deamon threads will likely
wedge the KPL, some retry logic or a hard shutdown should probably be
implemented in a subsequent commit.

(partially)
fixes #111

This change updates calls to ExecutorService.submit (mostly deamons), to
call ExecutorService.execute instead.  The submit variant returns a
Future which will contain any errors that are thrown from the Runnable.
However, nothing was checking these throwables for errors.  If an error
happens, for example an OutOfMemoryError or RuntimeException thrown from
a custom AWSCredentialsProvider, the deamon thread will die without ever
logging any error information.  Changing this to use execute allows the
Thread's UncaughtExceptionHandler or the default
UncaughtExceptionHandler to properly handle the failure.  At a minimum
this allows the error to be logged.  Some clients may wish to respond to
an OutOfMemoryError by taking a more severe action, such as restaring
the service.  Given that failure of these deamon threads will likely
wedge the KPL, some retry logic or a hard shutdown should probably be
implemented in a subsequent commit.
@jeremysears
Copy link
Contributor Author

I've updated the PR, so that it doesn't contain superfluous white-space changes.

@pfifer
Copy link
Contributor

pfifer commented Oct 3, 2017

Please confirm that we can use, modify, copy, and redistribute this contribution. Thanks.

@jeremysears
Copy link
Contributor Author

I confirm that you may use, modify, copy, and redistribute this change.

@pfifer pfifer merged commit fd6ee0c into awslabs:master Nov 13, 2017
@pfifer pfifer added this to the v0.12.6 milestone Nov 13, 2017
pfifer added a commit to pfifer/amazon-kinesis-producer that referenced this pull request Nov 14, 2017
* Added Windows support
  The 0.12.x version now supports Windows.
  The Windows version is currently mastered on the branch `windows`, which will be merged at a later date.
  The build instructions for Windows are currently out of date, and will be updated at a later date.__
  * Issue awslabs#113
  * Issue awslabs#74
  * Issue awslabs#73
* Removed the libc wrapper
  The libc wrapper lowered the required version of glibc.  The KPL is now built with an older version of libc, which removes the need for the wrapper.
  * PR awslabs#139
* Set the minimum required version of macOS to 10.9.
  The KPL is now built against macOS 10.9.
  * Issue awslabs#117
  * PR awslabs#138

* Allow exceptions to bubble to the thread exception handler for Daemon threads.
  Exceptions that occur on daemon threads will now be allowed to propagate to the thread exception handler.  This doesn't provide any additional monitoring or handling of thread death.
  * PR awslabs#112
  * Issue awslabs#111
* Updated `amazon-kinesis-producer-sample` to use the correct properties in its configuration file.
  * PR awslabs#120
  * Issue awslabs#119
* Updated documentation of `AggregationMaxSize` to match actual Kinesis limits.
  * PR awslabs#133
* Added support for setting `ThreadingModel`, and `ThreadPoolSize` using a properties file.
  * PR awslabs#134
  * Issue awslabs#124
* Extracted `IKinesisProducer` from `KinesisProducer` to allow for easier testing.
  * PR awslabs#136
pfifer added a commit that referenced this pull request Nov 14, 2017
* Added Windows support
  The 0.12.x version now supports Windows.
  The Windows version is currently mastered on the branch `windows`, which will be merged at a later date.
  The build instructions for Windows are currently out of date, and will be updated at a later date.__
  * Issue #113
  * Issue #74
  * Issue #73
* Removed the libc wrapper
  The libc wrapper lowered the required version of glibc.  The KPL is now built with an older version of libc, which removes the need for the wrapper.
  * PR #139
* Set the minimum required version of macOS to 10.9.
  The KPL is now built against macOS 10.9.
  * Issue #117
  * PR #138

* Allow exceptions to bubble to the thread exception handler for Daemon threads.
  Exceptions that occur on daemon threads will now be allowed to propagate to the thread exception handler.  This doesn't provide any additional monitoring or handling of thread death.
  * PR #112
  * Issue #111
* Updated `amazon-kinesis-producer-sample` to use the correct properties in its configuration file.
  * PR #120
  * Issue #119
* Updated documentation of `AggregationMaxSize` to match actual Kinesis limits.
  * PR #133
* Added support for setting `ThreadingModel`, and `ThreadPoolSize` using a properties file.
  * PR #134
  * Issue #124
* Extracted `IKinesisProducer` from `KinesisProducer` to allow for easier testing.
  * PR #136
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.

KPL Silently Swallows Exceptions from Deamon Threads
2 participants