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

NPE during NettyClientTransport.start() if keepAlive is enabled #2726

Closed
bobwenx opened this issue Feb 15, 2017 · 3 comments
Closed

NPE during NettyClientTransport.start() if keepAlive is enabled #2726

bobwenx opened this issue Feb 15, 2017 · 3 comments
Labels
Milestone

Comments

@bobwenx
Copy link

bobwenx commented Feb 15, 2017

What version of gRPC are you using?

GRPC 1.1.2

What JVM are you using (java -version)?

1.8.0_112

What did you do?

Just simply call io.grpc.netty.NettyChannelBuilder#enableKeepAlive(boolean) during channel build. and the grpc always throw NPE exception.

java.lang.NullPointerException: null
	at io.grpc.netty.NettyClientTransport.start(NettyClientTransport.java:169)
	at io.grpc.internal.ForwardingConnectionClientTransport.start(ForwardingConnectionClientTransport.java:44)
	at io.grpc.internal.TransportSet.startNewTransport(TransportSet.java:233)
	at io.grpc.internal.TransportSet.obtainActiveTransport(TransportSet.java:203)
	at io.grpc.internal.ManagedChannelImpl$3.getTransport(ManagedChannelImpl.java:739)
	at io.grpc.internal.ManagedChannelImpl$3.getTransport(ManagedChannelImpl.java:677)
	at io.grpc.PickFirstBalancerFactory$PickFirstBalancer$1.get(PickFirstBalancerFactory.java:129)
	at io.grpc.internal.DelayedClientTransport$2.run(DelayedClientTransport.java:271)
	at java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1402)
	at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
	at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
	at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
	at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157)

and inspect the code(io.grpc.netty.NettyClientTransport#start):

 @SuppressWarnings("unchecked")
  @Override
  public Runnable start(Listener transportListener) {
    lifecycleManager = new ClientTransportLifecycleManager(
        Preconditions.checkNotNull(transportListener, "listener"));

    if (enableKeepAlive) {
      keepAliveManager = new KeepAliveManager(this, channel.eventLoop(), keepAliveDelayNanos,
          keepAliveTimeoutNanos);
    }
    ....
}

i believe the field channel is never initialized right now, so NPE is throw by field call channel.eventLoop().

@ejona86
Copy link
Member

ejona86 commented Feb 15, 2017

Yeah... that looks broken. channel is initialized in the same method on line 197. @lukaszx0, what to make a fix, and figure out why a test didn't catch this?

@ejona86 ejona86 added the bug label Feb 15, 2017
@ejona86 ejona86 added this to the Next milestone Feb 15, 2017
@lukaszx0
Copy link
Collaborator

@ejona86 yes, I was planning to look into that as soon as I'll find some free time. Hopefully this week.

lukaszx0 added a commit to lukaszx0/grpc-java that referenced this issue Feb 15, 2017
* Move creation of keepAliveManager to the bottom of start()
* Enable keepAlive in NettyClientTransportTest
* Add test cases checking if keepalive is enabled/disabled, specifically.

Fixes grpc#2726
@bobwenx
Copy link
Author

bobwenx commented Feb 16, 2017

@ejona86 When is this fix be released? KeepAlive support is a important feature for us(and that's why we upgrade to 1.1.x), Hope this fix release quickly.

lukaszx0 added a commit to lukaszx0/grpc-java that referenced this issue Feb 16, 2017
Fixes NPE when keepalive is enabled.

* Move creation of keepAliveManager to the bottom of start()
* Enable keepAlive in NettyClientTransportTest
* Add test cases checking if keepalive is enabled/disabled, specifically.

Fixes grpc#2726
ejona86 pushed a commit that referenced this issue Feb 16, 2017
Fixes NPE when keepalive is enabled.

* Move creation of keepAliveManager to the bottom of start()
* Enable keepAlive in NettyClientTransportTest
* Add test cases checking if keepalive is enabled/disabled, specifically.

Fixes #2726
lukaszx0 added a commit to lukaszx0/grpc-java that referenced this issue Feb 17, 2017
Fixes NPE when keepalive is enabled.

* Move creation of keepAliveManager to the bottom of start()
* Enable keepAlive in NettyClientTransportTest
* Add test cases checking if keepalive is enabled/disabled, specifically.

Fixes grpc#2726 (Backports grpc#2729)
ejona86 pushed a commit that referenced this issue Feb 17, 2017
Fixes NPE when keepalive is enabled.

* Move creation of keepAliveManager to the bottom of start()
* Enable keepAlive in NettyClientTransportTest
* Add test cases checking if keepalive is enabled/disabled, specifically.

Fixes #2726 (Backports #2729)
@ejona86 ejona86 modified the milestones: 1.2, Next Apr 5, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Sep 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants