GrpcTransportOptions uses internal grpc APIs

[ejona86]

This is reopening #120 which should not have been closed. Internal APIs change without warning, which would break your users. Please remove the usage of the internal APIs.

java-core/google-cloud-core-grpc/src/main/java/com/google/cloud/grpc/GrpcTransportOptions.java

Lines 35 to 36 in 13414b6

	import io.grpc.internal.SharedResourceHolder;
	import io.grpc.internal.SharedResourceHolder.Resource;

[suztomo]

Memo: that's the only places in this repository that touch io.grpc.internal package:

~/java-core $ grep -ir 'import io.grpc.internal' .                                                       git[branch:main]
./google-cloud-core-grpc/src/main/java/com/google/cloud/grpc/GrpcTransportOptions.java:import io.grpc.internal.SharedResourceHolder;
./google-cloud-core-grpc/src/main/java/com/google/cloud/grpc/GrpcTransportOptions.java:import io.grpc.internal.SharedResourceHolder.Resource;

The lines were added in 2016 commit 827a94d9 in old repository googleapis/google-cloud-java#1011

Todo on me:

• To understand how the EXECUTOR (which is a io.grpc.internal.SharedResourceHolder.Resource) helps the user of GrpcTransportOptions class.

     /** Shared thread pool executor. */
     private static final Resource<ScheduledExecutorService> EXECUTOR =
         new Resource<ScheduledExecutorService>() {
  

  -> In fix: stop using internal gRPC classes #129 Jeff wrote "SharedResourceHolder is essentially a thread-safe, lazy-initializing, singleton registry for any Resource (something that can be shutdown/closed)."
• Confirm the internal classes are not leaked to the return value of public methods of GrpcTransportOptions class.
  -> Not leaked.

[ejona86]

What sort of timeframe does p3 mean this might get fixed? I opened the original issue 3.5 years ago. P3 makes me worry it will be another 3 years and nothing will have happened here. Given this is a diamond dependency danger, it seems inappropriate to be p3.

If this is fixed tomorrow, it will still be an issue for another year-ish afterward, as users should be able to upgrade their grpc version at-will and they may be using an older java-core version.

[suztomo]

@ejona86 I'm afraid p3 is not high priority and it may not match the urgency you have in mind. Do you have a plan to remove or restrict the access of theio.grpc.internal.SharedResourceHolder? It may change the priority in our team.

[suztomo]

@chingor13 I see #129 might fix the issue. What was the problem that prevented you from merging it?

[suztomo]

@ejona86 We will discuss this in a team meeting and will get back to you in few days.