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.

Sign up for GitHub

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

Jump to bottom

NPE in TemplateInitializer when creating instance from background thread #3738

Closed
OlliTietavainenVaadin opened this issue Mar 21, 2018 · 15 comments
Closed

NPE in TemplateInitializer when creating instance from background thread #3738

OlliTietavainenVaadin opened this issue Mar 21, 2018 · 15 comments
Milestone
Abandoned

Comments

@OlliTietavainenVaadin
Copy link
Member

1.0.0.beta3 with Spring

Based on this Forum thread: https://vaadin.com/forum/thread/17015671

I'm not clear on an exact mechanism on how this can be reproduced. The main idea is that in certain conditions, when creating an instance of a custom PolymerTemplate component in a background thread, TemplateInitializer will fail with a NPE. The exception seems to be thrown from this line in the constructor:

        boolean productionMode = VaadinService.getCurrent()
                .getDeploymentConfiguration().isProductionMode();

Guess: VaadinService.getCurrent() returns null here.

Whether or not the application itself and/or thread handling needs to be implemented differently can be debated, but at least TemplateInitializer should fail more gracefully with a helpful error message.

Stack trace:

java.lang.NullPointerException: null
	at com.vaadin.flow.component.polymertemplate.TemplateInitializer.<init>(TemplateInitializer.java:71)
	at com.vaadin.flow.component.polymertemplate.PolymerTemplate.<init>(PolymerTemplate.java:72)
	at com.vaadin.flow.component.polymertemplate.PolymerTemplate.<init>(PolymerTemplate.java:87)
	at com.avides.waresca.verify.ui.view.instruction.ScannerRepresenter.<init>(ScannerRepresenter.java:43)
	at com.avides.waresca.verify.ui.view.instruction.DefaultInstructionView.lambda$updateRegisteredScannerContainer$0(DefaultInstructionView.java:95)
	at java.lang.Iterable.forEach(Iterable.java:75)
	at com.avides.waresca.verify.ui.view.instruction.DefaultInstructionView.updateRegisteredScannerContainer(DefaultInstructionView.java:95)
	at com.avides.waresca.verify.ui.view.instruction.DefaultInstructionView.displayLoginSuccessful(DefaultInstructionView.java:87)
	at com.avides.waresca.verify.ui.presenter.instruction.InstructionPresenter.handleLoginInstruction(InstructionPresenter.java:82)
	at com.avides.waresca.verify.ui.presenter.instruction.InstructionPresenter.handle(InstructionPresenter.java:67)
	at com.avides.waresca.verify.ui.presenter.instruction.InstructionPresenter.handle(InstructionPresenter.java:22)
	at com.avides.waresca.verify.ui.presenter.instruction.InstructionPresenter$$FastClassBySpringCGLIB$$8a48e722.invoke(<generated>)
	at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:204)
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint(CglibAopProxy.java:747)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
	at org.springframework.validation.beanvalidation.MethodValidationInterceptor.invoke(MethodValidationInterceptor.java:112)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185)
	at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:689)
	at com.avides.waresca.verify.ui.presenter.instruction.InstructionPresenter$$EnhancerBySpringCGLIB$$37ee290f.handle(<generated>)
	at com.avides.waresca.verify.listener.websocket.InstructionWebSocketListener.handle(InstructionWebSocketListener.java:28)
	at com.avides.waresca.verify.listener.websocket.InstructionWebSocketListener.handle(InstructionWebSocketListener.java:12)
	at com.avides.waresca.verify.listener.websocket.AbstractWebSocketListener.handleFrame(AbstractWebSocketListener.java:34)
	at com.avides.waresca.verify.listener.websocket.AbstractWebSocketListener$$FastClassBySpringCGLIB$$3cd55b30.invoke(<generated>)
	at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:204)
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint(CglibAopProxy.java:747)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
	at org.springframework.validation.beanvalidation.MethodValidationInterceptor.invoke(MethodValidationInterceptor.java:112)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185)
	at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:689)
	at com.avides.waresca.verify.listener.websocket.InstructionWebSocketListener$$EnhancerBySpringCGLIB$$d712160b.handleFrame(<generated>)
	at org.springframework.messaging.simp.stomp.DefaultStompSession.invokeHandler(DefaultStompSession.java:462)
	at org.springframework.messaging.simp.stomp.DefaultStompSession.handleMessage(DefaultStompSession.java:409)
	at org.springframework.web.socket.messaging.WebSocketStompClient$WebSocketTcpConnectionHandlerAdapter.handleMessage(WebSocketStompClient.java:344)
	at org.springframework.web.socket.adapter.standard.StandardWebSocketHandlerAdapter.handleTextMessage(StandardWebSocketHandlerAdapter.java:113)
	at org.springframework.web.socket.adapter.standard.StandardWebSocketHandlerAdapter.access$000(StandardWebSocketHandlerAdapter.java:42)
	at org.springframework.web.socket.adapter.standard.StandardWebSocketHandlerAdapter$3.onMessage(StandardWebSocketHandlerAdapter.java:84)
	at org.springframework.web.socket.adapter.standard.StandardWebSocketHandlerAdapter$3.onMessage(StandardWebSocketHandlerAdapter.java:81)
	at org.apache.tomcat.websocket.WsFrameBase.sendMessageText(WsFrameBase.java:395)
	at org.apache.tomcat.websocket.WsFrameBase.processDataText(WsFrameBase.java:495)
	at org.apache.tomcat.websocket.WsFrameBase.processData(WsFrameBase.java:294)
	at org.apache.tomcat.websocket.WsFrameBase.processInputBuffer(WsFrameBase.java:133)
	at org.apache.tomcat.websocket.WsFrameClient.processSocketRead(WsFrameClient.java:95)
	at org.apache.tomcat.websocket.WsFrameClient.resumeProcessing(WsFrameClient.java:209)
	at org.apache.tomcat.websocket.WsFrameClient.access$300(WsFrameClient.java:31)
	at org.apache.tomcat.websocket.WsFrameClient$WsFrameClientCompletionHandler.doResumeProcessing(WsFrameClient.java:186)
	at org.apache.tomcat.websocket.WsFrameClient$WsFrameClientCompletionHandler.completed(WsFrameClient.java:163)
	at org.apache.tomcat.websocket.WsFrameClient$WsFrameClientCompletionHandler.completed(WsFrameClient.java:148)
	at sun.nio.ch.Invoker.invokeUnchecked(Invoker.java:126)
	at sun.nio.ch.Invoker$2.run(Invoker.java:218)
	at sun.nio.ch.AsynchronousChannelGroupImpl$1.run(AsynchronousChannelGroupImpl.java:112)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
@denis-anisimov
Copy link
Contributor

denis-anisimov commented Mar 21, 2018 •
edited
Loading

The ticket says actually:

Whether or not the application itself and/or thread handling needs to be implemented differently can be debated, but at least TemplateInitializer should fail more gracefully with a helpful error message.

That what needs to be fixed.

If VaadinService.getCurrent() returns null then it means that the execution is done in the custom thread without a proper synchronization.
VaadinService.getCurrent() is available if the code is executed inside servlet request dispatcher thread and it also available when the code is executed via the UI.access() which is the proper way to execute any UI related code.
In all other situations VaadinService should be set via setCurrent explicitly be the developer (and in most such cases it means that the code is executed without a required session lock which is incorrect).

@OlliTietavainenVaadin
Copy link
Member Author

At least based on the thread, this occurs even when executing the code via UI.access().

@denis-anisimov
Copy link
Contributor

The exact source code is needed then.

@denis-anisimov
Copy link
Contributor

The stacktrace in the main description is bad.
It clearly shows that exception happens inside custom thread without any UI.access().

The good stacktrace is (from the forum thread):

java.util.concurrent.ExecutionException: java.lang.NullPointerException
	at java.util.concurrent.FutureTask.report(FutureTask.java:122)
	at java.util.concurrent.FutureTask.get(FutureTask.java:192)
	at com.vaadin.flow.server.FutureAccess.get(FutureAccess.java:61)
	at com.vaadin.flow.server.VaadinService.runPendingAccessTasks(VaadinService.java:1972)
	at com.vaadin.flow.server.VaadinSession.unlock(VaadinSession.java:621)
	at com.vaadin.flow.server.VaadinService.ensureAccessQueuePurged(VaadinService.java:1934)
	at com.vaadin.flow.server.VaadinService.accessSession(VaadinService.java:1900)
	at com.vaadin.flow.server.VaadinSession.access(VaadinSession.java:923)
	at com.vaadin.flow.component.UI.access(UI.java:444)
	at com.avides.waresca.verify.ui.presenter.instruction.InstructionPresenter.handle(InstructionPresenter.java:70)
	at com.avides.waresca.verify.ui.presenter.instruction.InstructionPresenter.handle(InstructionPresenter.java:24)
	at com.avides.waresca.verify.ui.presenter.instruction.InstructionPresenter$$FastClassBySpringCGLIB$$8a48e722.invoke(<generated>)
	at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:204)
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint(CglibAopProxy.java:747)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
	at org.springframework.validation.beanvalidation.MethodValidationInterceptor.invoke(MethodValidationInterceptor.java:112)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185)
	at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:689)
	at com.avides.waresca.verify.ui.presenter.instruction.InstructionPresenter$$EnhancerBySpringCGLIB$$e66e4481.handle(<generated>)
	at com.avides.waresca.verify.listener.websocket.InstructionWebSocketListener.handle(InstructionWebSocketListener.java:28)
	at com.avides.waresca.verify.listener.websocket.InstructionWebSocketListener.handle(InstructionWebSocketListener.java:12)
	at com.avides.waresca.verify.listener.websocket.AbstractWebSocketListener.handleFrame(AbstractWebSocketListener.java:34)
	at com.avides.waresca.verify.listener.websocket.AbstractWebSocketListener$$FastClassBySpringCGLIB$$3cd55b30.invoke(<generated>)
	at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:204)
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint(CglibAopProxy.java:747)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
	at org.springframework.validation.beanvalidation.MethodValidationInterceptor.invoke(MethodValidationInterceptor.java:112)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:185)
	at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:689)
	at com.avides.waresca.verify.listener.websocket.InstructionWebSocketListener$$EnhancerBySpringCGLIB$$8592317d.handleFrame(<generated>)
	at org.springframework.messaging.simp.stomp.DefaultStompSession.invokeHandler(DefaultStompSession.java:462)
	at org.springframework.messaging.simp.stomp.DefaultStompSession.handleMessage(DefaultStompSession.java:409)
	at org.springframework.web.socket.messaging.WebSocketStompClient$WebSocketTcpConnectionHandlerAdapter.handleMessage(WebSocketStompClient.java:344)
	at org.springframework.web.socket.adapter.standard.StandardWebSocketHandlerAdapter.handleTextMessage(StandardWebSocketHandlerAdapter.java:113)
	at org.springframework.web.socket.adapter.standard.StandardWebSocketHandlerAdapter.access$000(StandardWebSocketHandlerAdapter.java:42)
	at org.springframework.web.socket.adapter.standard.StandardWebSocketHandlerAdapter$3.onMessage(StandardWebSocketHandlerAdapter.java:84)
	at org.springframework.web.socket.adapter.standard.StandardWebSocketHandlerAdapter$3.onMessage(StandardWebSocketHandlerAdapter.java:81)
	at org.apache.tomcat.websocket.WsFrameBase.sendMessageText(WsFrameBase.java:395)
	at org.apache.tomcat.websocket.WsFrameBase.processDataText(WsFrameBase.java:495)
	at org.apache.tomcat.websocket.WsFrameBase.processData(WsFrameBase.java:294)
	at org.apache.tomcat.websocket.WsFrameBase.processInputBuffer(WsFrameBase.java:133)
	at org.apache.tomcat.websocket.WsFrameClient.processSocketRead(WsFrameClient.java:95)
	at org.apache.tomcat.websocket.WsFrameClient.resumeProcessing(WsFrameClient.java:209)
	at org.apache.tomcat.websocket.WsFrameClient.access$300(WsFrameClient.java:31)
	at org.apache.tomcat.websocket.WsFrameClient$WsFrameClientCompletionHandler.doResumeProcessing(WsFrameClient.java:186)
	at org.apache.tomcat.websocket.WsFrameClient$WsFrameClientCompletionHandler.completed(WsFrameClient.java:163)
	at org.apache.tomcat.websocket.WsFrameClient$WsFrameClientCompletionHandler.completed(WsFrameClient.java:148)
	at sun.nio.ch.Invoker.invokeUnchecked(Invoker.java:126)
	at sun.nio.ch.Invoker$2.run(Invoker.java:218)
	at sun.nio.ch.AsynchronousChannelGroupImpl$1.run(AsynchronousChannelGroupImpl.java:112)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.NullPointerException: null
	at com.vaadin.flow.component.polymertemplate.
(DefaultTemplateParser.java:84)
	at com.vaadin.flow.component.polymertemplate.TemplateDataAnalyzer.parseTemplate(TemplateDataAnalyzer.java:179)
	at com.vaadin.flow.component.polymertemplate.TemplateInitializer.<init>(TemplateInitializer.java:87)
	at com.vaadin.flow.component.polymertemplate.PolymerTemplate.<init>(PolymerTemplate.java:72)
	at com.vaadin.flow.component.polymertemplate.PolymerTemplate.<init>(PolymerTemplate.java:87)
	at com.avides.waresca.verify.ui.view.instruction.ScannerRepresenter.<init>(ScannerRepresenter.java:43)
	at com.avides.waresca.verify.ui.view.instruction.DefaultInstructionView.lambda$updateRegisteredScannerContainer$0(DefaultInstructionView.java:89)
	at java.lang.Iterable.forEach(Iterable.java:75)
	at com.avides.waresca.verify.ui.view.instruction.DefaultInstructionView.updateRegisteredScannerContainer(DefaultInstructionView.java:89)
	at com.avides.waresca.verify.ui.view.instruction.DefaultInstructionView.displayLoginSuccessful(DefaultInstructionView.java:81)
	at com.avides.waresca.verify.ui.presenter.instruction.InstructionPresenter.handleLoginInstruction(InstructionPresenter.java:90)
	at com.avides.waresca.verify.ui.presenter.instruction.InstructionPresenter.lambda$handle$9f3cb4ad$1(InstructionPresenter.java:74)
	at com.vaadin.flow.component.UI.accessSynchronously(UI.java:383)
	at com.vaadin.flow.component.UI$1.execute(UI.java:447)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at com.vaadin.flow.server.VaadinService.runPendingAccessTasks(VaadinService.java:1969)

This shows that the exception happens inside UI.access() and this is genuine bug.

But this is totally different exception which is thrown inside DefaultTemplateParser.getTemplateContent and node inside TemplateInitializer.
So it looks like there are two different issues in the same bug.
This is not good.
Another ticket needs to be created.

@Legioth
Copy link
Member

Legioth commented Mar 23, 2018

A good question is whether it should necessary to be inside UI.access() when only creating a component instance.

It will definitely be required when the created component gets attached to the UI, but anything before that is somewhat of a gray area.

@denis-anisimov
Copy link
Contributor

A good question is whether it should necessary to be inside UI.access() when only creating a component instance.

Yes, of course.
EVERY action with any UI object has to be done under the lock.
Otherwise it's not thread safe. In the same way as any Swing component has to handled only inside AWT thread (including instantiation).

@Legioth
Copy link
Member

Legioth commented Mar 23, 2018

So far, anything that hasn't been directly connected to any specific VaadinSession instance has been working fine simply since any instance that isn't yet attached won't even be referenced from anything running on a different thread (unless the application logic itself publishes a reference).

Even though it might still have been semantically wrong to do so, it has worked without problems for many years.

@denis-anisimov
Copy link
Contributor

The common pattern in Swing is to have a CTOR with a parent component as an argument.
Usually CTOR adds the component to its parent.
That's it. This operation is not thread safe.

It's only theoretically possible to create a UI instance out of the UI thread without any problem and then attach it to the UI tree inside UI thread.
In reality in most cases any UI object creation may be very dangerous because of thread safety.

I've seen quite a lot of cases where this happens in Swing.

That's why there is a rule of dumb: any UI object has to be scoped inside UI thread and may not leak to outside of it.
The good designed API should not even allow to do any action with any UI object outside of lock/UI thread.

@hnrkdmsk
Copy link

Hello, I have an example for you thats represents the bug. I have tried with a Broadcaster and without the Broadcaster. Both end with this exception.
vaadin_push.zip

@Legioth
Copy link
Member

Legioth commented Mar 28, 2018 •
edited
Loading

@Henrizzle The only exception I see when doing mvn spring-boot:run for the attached vaadin_push.zip and clicking around in the application in Chrome is java.lang.IllegalStateException: Can't find resource 'frontend://src/example-template.html' via the servlet context. How is that exception related to this issue?

@hnrkdmsk
Copy link

Disable the Broadcaster and then you will see the same exception...

Use this to find this bug. I think it stands in relation with Spring Boot 2?

@Legioth
Copy link
Member

Legioth commented Mar 28, 2018

@Henrizzle Could you please share code or very specific instructions that actually reproduces the issue instead of only giving a rather vague description on how to modify the code? Even if I would make a guess that leads to a situation that looks similar to the NullPointerException that this ticket is about, I could still not be sure that it's the same case that you are having.

This was referenced Mar 28, 2018
Closed
Closed
@hnrkdmsk
Copy link

See https://github.com/henrizzle/vaadin-flow-push-example

@Legioth
Copy link
Member

Legioth commented Mar 29, 2018

The example code modifies the UI without using UI.access, i.e. without properly protecting the UI state against concurrent modifications from multiple threads. A side effect of not using UI.access is also that VaadinService.getCurrent() isn't defined, which causes the observed NullPointerException.

I will close this issue and create a new one that is only focused on the other NullPointerException that happens despite proper UI.access usage from #3738 (comment)

@Legioth Legioth closed this as completed Mar 29, 2018
@Legioth Legioth mentioned this issue Mar 29, 2018
Open
@Legioth
Copy link
Member

Legioth commented Mar 29, 2018

Created #3789 about the NullPointerException that didn't have an explanation and #3790 about making it easier to debug cases when the session isn't properly locked.

@denis-anisimov denis-anisimov added this to the Abandoned milestone Mar 29, 2018
@denis-anisimov denis-anisimov mentioned this issue Apr 18, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Abandoned
Development

No branches or pull requests
4 participants
@Legioth @hnrkdmsk @denis-anisimov @OlliTietavainenVaadin