'channelLogger' is already in use when updating from 1.23.x to 1.24.x #6707
Comments
|
the exception can only happen if the static final AtomicBoolean block = new AtomicBoolean();
@Test
public void testNullKey() throws Exception {
if (block.compareAndSet(false, true)) {
return;
}
// original test case
// ...
}
oops ignore previous message, but the problem is the static block is evaluated more than one time. also noticed that the test is executed multiple times. |
|
this is most likely due to your test environment. in 1.24.0, we added a handler that access |
|
@creamsoup you call it "messed up", I call it "ClassLoader isolation" 😜 There are indeed two separate ClassLoaders for gRPC - once inside the application under test, and another one to test the application. Since the logger key does not seem to be that critical, perhaps you can gracefully handle such situation? Thanks! |
|
oops sorry to call it messed up 😉 I believe throwing is the right behavior because applications can pass their own attributes with the same key. In this case, two same name keys most likely will have two different class types, therefore it can cause runtime exception. Gracefully handling this is error prone. The |
|
@creamsoup no problem :)
I would like to mention that (unlike gRPC), Netty requires some native dependencies that cannot be isolated with classloaders. Also, as a high performant framework, it is better to reuse the global resources created by Netty. As much as I would like to not have it shared, it is almost impossible.
Can the key be randomized or include the classloader's id? |
if there is a really good reason to do so, but purely for a very specific test scenario is not a very compelling reason. |
|
This is a problem. At the very least it would break using grpc-netty and grpc-netty-shaded at the same time. Note that even without shading it is possible to load gRPC multiple times, simultaneously, from different class loaders as done in the test. We would want that to work. To make sure "it is only us" using the AttributeKey we should use NettyClientTransport.class.getName() to make sure it is "unique to us." But we will need to coordinate with other instances of our classes (from other class loaders) to re-use instances. This is needed for two reasons:
Since gRPC controls the Netty Channel, it looks safe to share the AttributeKey with other gRPC instances in other class loaders; there is no way for them to mix. But even better: we should consider deleting the attribute. It is only used by ProtocolNegotiators, which already has access to GrpcHttp2ConnectionHandler. We could just add method to GrpcHttp2ConnectionHandler instead, which would be explicit. |
Singletons/constants are per-classloader. You can generate a random key and it will be constant, but only for this classloader. |
Ah, yes, true. Edit: Ah, yes, I also see a mention of something like that in #7048. "If this causes issues, this AttributeKey should be updated to use a new UUID (or something) each time it is loaded." I hadn't gotten to reading that yet :) |
I realized this isn't actually true, because netty-shaded has a separate copy of Netty and thus a different static pool! Still, we do need to work in ClassLoader environments like this. |
What version of gRPC-Java are you using?
1.124.0
What is your environment?
Mac OS X
Java 11
What did you expect to see?
No error
What did you see instead?
java.lang.IllegalArgumentException: 'channelLogger' is already in use
Steps to reproduce the bug
AbstractIntegrationTestinapp/src/test/javaand setuseGrpcto trueSmokeTesteither by delegating to Gradle in your IDE or with GradleThe text was updated successfully, but these errors were encountered: