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

ExecutorService exception handling #40

Open
tpietzsch opened this issue May 16, 2017 · 2 comments
Open

ExecutorService exception handling #40

tpietzsch opened this issue May 16, 2017 · 2 comments

Comments

@tpietzsch
Copy link
Member

In several places we have code like this:

for ( final Future< Void > f : futures )
{
	try
	{
		f.get();
	}
	catch ( final InterruptedException e )
	{
		e.printStackTrace();
	}
	catch ( final ExecutionException e )
	{
		e.printStackTrace();
	}
}

This should be changed from e.printStackTrace() to something meaningful.

InterruptedException should Thread.currentThread().interrupt() and end method as fast as possible.

What should we do with ExecutionException? Just declare and throw it?

@ctrueden
Copy link
Member

What should we do with ExecutionException? Just declare and throw it?

Makes sense to me, where possible. This method is waiting for code to execute, after all. Shouldn't it report when things go wrong?

@hanslovsky
Copy link
Member

I fully agree. Should we provide a convenience method that does exactly that so people have an incentive to do that.
Something like

public static < T > List< T > waitForExecution(  List< Future< T > > futures )
throws ExecutionException, InterruptedException
{
	ArrayList< T > result = new ArrayList<>();
	for ( final Future< Void > f : futures )
	{
		try
		{
			result.add( f.get() );
		}
		catch ( final InterruptedException e )
		{
			Thread.currentThread().interrupt();
			throw e; // re-throw?
		}
	}
	return result;
}

What about ExecutorService.invokeAll throws InterruptedException? That should probably be handled in a similar fashion.

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

No branches or pull requests

3 participants