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

ClassCastException in AbstractPollingEndpoint #3418

Closed
toddlindner opened this issue Oct 30, 2020 · 9 comments · Fixed by #3419
Closed

ClassCastException in AbstractPollingEndpoint #3418

toddlindner opened this issue Oct 30, 2020 · 9 comments · Fixed by #3419
Assignees
Labels
status: waiting-for-triage The issue need to be evaluated and its future decided type: bug
Milestone

Comments

@toddlindner
Copy link

toddlindner commented Oct 30, 2020

Spring Integration 5.3.2.RELEASE

I am seeing a ClassCastException during error handling in AbstractPollingEndpoint. Using jdk15 so there is more detail on the ClassCastException.

The thing is, looking at this line

For the life of me, I can't understand how a non RuntimeException can make it here. In my case I am getting java.nio.file.NoSuchFileException. Understand this is not an ideal bug report, but as a pure Java dev, I still cant figure out how a non-runtime exception can make it to line 417. The receiveMessage() method above has no throws declared so the exception thrown must be a RuntimeException, but java.nio.file.NoSuchFileException is not. The only thing I can think of would be some sort of lombok @SneakyThrows somewhere but that would still create a UndeclaredThrowableException around the inner exception.

I am not trying to stop the exception (I assume a user had a file locked during polling or something similar), I just want to get a normal throw without a ClassCastException.

This error only happens about once per day so it is difficult to reproduce. The app has about 10 pollers but they are all similar. Unfortunately I don't know which one is failing since there is no other information in the log or the stack trace regarding bean id.

The spring configuraiton xml is using this adapter <int-file:inbound-channel-adapter>

2020-10-29 22:19:09,833 [scheduling-1] ERROR    o.s.i.handler.LoggingHandler - org.springframework.messaging.MessagingException: nested exception is java.lang.ClassCastException: class java.nio.file.NoSuchFileException cannot be cast to class java.lang.RuntimeException (java.nio.file.NoSuchFileException and java.lang.RuntimeException are in module java.base of loader 'bootstrap')
	at org.springframework.integration.endpoint.AbstractPollingEndpoint.pollForMessage(AbstractPollingEndpoint.java:390)
	at org.springframework.integration.endpoint.AbstractPollingEndpoint.lambda$null$3(AbstractPollingEndpoint.java:323)
	at org.springframework.integration.util.ErrorHandlingTaskExecutor.lambda$execute$0(ErrorHandlingTaskExecutor.java:57)
	at org.springframework.core.task.SyncTaskExecutor.execute(SyncTaskExecutor.java:50)
	at org.springframework.integration.util.ErrorHandlingTaskExecutor.execute(ErrorHandlingTaskExecutor.java:55)
	at org.springframework.integration.endpoint.AbstractPollingEndpoint.lambda$createPoller$4(AbstractPollingEndpoint.java:320)
	at org.springframework.scheduling.support.DelegatingErrorHandlingRunnable.run(DelegatingErrorHandlingRunnable.java:54)
	at org.springframework.scheduling.concurrent.ReschedulingRunnable.run(ReschedulingRunnable.java:93)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
	at java.base/java.lang.Thread.run(Thread.java:832)
Caused by: java.lang.ClassCastException: class java.nio.file.NoSuchFileException cannot be cast to class java.lang.RuntimeException (java.nio.file.NoSuchFileException and java.lang.RuntimeException are in module java.base of loader 'bootstrap')
	at org.springframework.integration.endpoint.AbstractPollingEndpoint.doPoll(AbstractPollingEndpoint.java:417)
	at org.springframework.integration.endpoint.AbstractPollingEndpoint.pollForMessage(AbstractPollingEndpoint.java:376)
	... 13 more
@toddlindner toddlindner added status: waiting-for-triage The issue need to be evaluated and its future decided type: bug labels Oct 30, 2020
@artembilan
Copy link
Member

Well, having that ClassCastException it is not clear from where such a NoSuchFileException is thrown, but looking into the AbstractPollingEndpoint.doPoll() code, I think we are wrong:

	try {
			message = receiveMessage();
		}
		catch (Exception e) {
			if (Thread.interrupted()) {
				if (logger.isDebugEnabled()) {
					logger.debug("Poll interrupted - during stop()? : " + e.getMessage());
				}
				return null;
			}
			else {
				throw (RuntimeException) e;
			}
		}

we cast a caught Exception into a runtime one unconditionally.
Probably we must not do that independently of the root of the cause, even if it is not a part of method contract we are invoking.
Well, I think the regular compiled-in code flow may be broken somehow when we use reflection for method invocation...

It looks like ReflectionUtils.rethrowRuntimeException() should do the trick for us here to avoid ClassCastException.

Let me know what do you think and we will proceed with the fix!

@garyrussell
Copy link
Contributor

we cast a caught Exception

Right but receiveMessage() can only throw RTEs.

Something definitely odd. I do remember talking to Juergen a few years ago; there are some strange corner cases where this can happen (I seem to recall it had something to do with Constructors, but I don't remember the details).

@garyrussell
Copy link
Contributor

garyrussell commented Oct 30, 2020

Found it - using Class.newInstance() on a CTOR that throws a checked exception can cause the exception to be propagated from a method that has no throws clause.

Here's a discussion.

It has been deprecated since Java 9 for this reason:

     * @deprecated This method propagates any exception thrown by the
     * nullary constructor, including a checked exception.  Use of
     * this method effectively bypasses the compile-time exception
     * checking that would otherwise be performed by the compiler.
     * The {@link
     * java.lang.reflect.Constructor#newInstance(java.lang.Object...)
     * Constructor.newInstance} method avoids this problem by wrapping
     * any exception thrown by the constructor in a (checked) {@link
     * java.lang.reflect.InvocationTargetException}.

@artembilan
Copy link
Member

Right but receiveMessage() can only throw RTEs.

Correct. That's why I suggest to use a ReflectionUtils.rethrowRuntimeException(). See it's impl.

We probably have just never seen this problem because we are mostly on Java 8 yet.

@garyrussell
Copy link
Contributor

garyrussell commented Oct 30, 2020

I reproduced it quite easily; I don't see any such code in the framework.

@toddlindner - if you want to track down the root cause (where the actual exception is thrown), you can add this advice to the poller:

private ReceiveMessageAdvice errorTrapper() {
	return new ReceiveMessageAdvice() {

		@Override
		@Nullable
		public Object invoke(MethodInvocation invocation) throws Throwable {
			try {
				return ReceiveMessageAdvice.super.invoke(invocation);
			}
			catch (RuntimeException e) {
				throw e;
			}
			catch (Exception e) {
				throw new IllegalStateException(e);
			}
		}

		@Override
		@Nullable
		public Message<?> afterReceive(@Nullable Message<?> result, Object source) {
			return result;
		}

	};
}

This assumes SI 5.3 or later, for earlier versions (4.2 or later) you would have to extend AbstractMessageSourceAdvice.

Here's the complete test app:

@SpringBootApplication
public class Sigh3418Application {


	private static final Logger LOG = LoggerFactory.getLogger(Sigh3418Application.class);


	public static void main(String[] args) {
		SpringApplication.run(Sigh3418Application.class, args);
	}

	@Bean
	public IntegrationFlow flow() {
		return IntegrationFlows.from(new MyMS(), e -> e.poller(Pollers.fixedDelay(5000).advice(errorTrapper())))
				.nullChannel();
 	}

	private ReceiveMessageAdvice errorTrapper() {
		return new ReceiveMessageAdvice() {

			@Override
			@Nullable
			public Object invoke(MethodInvocation invocation) throws Throwable {
				try {
					return ReceiveMessageAdvice.super.invoke(invocation);
				}
				catch (RuntimeException e) {
					throw e;
				}
 				catch (Exception e) {
					throw new IllegalStateException(e);
				}
			}

			@Override
			@Nullable
			public Message<?> afterReceive(@Nullable Message<?> result, Object source) {
				return result;
			}

		};
	}

}

class MyMS implements MessageSource<String> {

	@Override
	@Nullable
	public Message<String> receive() {
		try {
			Foo.class.newInstance();
		}
		catch (InstantiationException | IllegalAccessException e) {
		}
		return null;
	}

}

class Foo {

	public Foo() throws IOException {
		throw new IOException();
	}

}

@garyrussell
Copy link
Contributor

We probably have just never seen this problem because we are mostly on Java 8 yet.

?? It fails with Java8 the same way.

@toddlindner
Copy link
Author

Thanks guys. Wow good catch Juergen and Gary on the Class.newInstance() issue. I never would have thought that was possible.
Agree ReflectionUtils.rethrowRuntimeException() seems like the cleanest fix here.

@artembilan
Copy link
Member

So, @garyrussell , I assign this to you because it looks like you are fully on board with the problem.

Thanks

garyrussell added a commit to garyrussell/spring-integration that referenced this issue Oct 30, 2020
Resolves spring-projects#3418

`Class.newInstance()` can propagate checked exceptions that are not declared.

**cherry-pick/back-port to 5.3.x, 5.2.x, 4.3.x**
artembilan pushed a commit that referenced this issue Nov 2, 2020
Resolves #3418

`Class.newInstance()` can propagate checked exceptions that are not declared.

**cherry-pick/back-port to 5.3.x, 5.2.x, 4.3.x**
artembilan pushed a commit that referenced this issue Nov 2, 2020
Resolves #3418

`Class.newInstance()` can propagate checked exceptions that are not declared.

**cherry-pick/back-port to 5.3.x, 5.2.x, 4.3.x**
artembilan pushed a commit that referenced this issue Nov 2, 2020
Resolves #3418

`Class.newInstance()` can propagate checked exceptions that are not declared.

**cherry-pick/back-port to 5.3.x, 5.2.x, 4.3.x**

# Conflicts:
#	spring-integration-core/src/main/java/org/springframework/integration/endpoint/AbstractPollingEndpoint.java
artembilan pushed a commit that referenced this issue Nov 2, 2020
Resolves #3418

`Class.newInstance()` can propagate checked exceptions that are not declared.

**cherry-pick/back-port to 5.3.x, 5.2.x, 4.3.x**
@toddlindner
Copy link
Author

Now that I am running the release I found the root cause of this. I had been using lombok @SneakyThrows on the accept() method of a FileListFilter, and it was throwing java.lang.reflect.UndeclaredThrowableException

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage The issue need to be evaluated and its future decided type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants