Support ExecutorService.close in ForwardingExecutorService

[findepi]

JDK's ExecutorService is AutoCloseable since Java 19.
ForwardingExecutorService does not forward the close method.

Forward close too, when running on Java 19.

[findepi]

It's not hard to implement that -- I can file a PR -- but obviously not without reflection.

i searched through https://github.com/google/guava/wiki and https://github.com/google/guava/wiki/CollectionHelpersExplained to understand what's the project policy on such cases, but didn't find an answer (apologies, if i just overlooked it!).
I also searched open issues and PRs for "ExecutorService close" and "19", but there too didn't find a prior discussion.

@cpovirk is there a project policy on how to update Forwarding* classes when newer Java version has new methods?

[cpovirk]

Our approach so far has been not to forward default methods (except for the case in which a method already existed and merely became a default method afterward).

In the case of ExecutorService.close, the motivation for not forwarding is that someone might use ForwardingExecutorService to implement an UnshutdownableExecutorService by overriding shutdown and shutdownNow. If ForwardingExecutorService began to forward close calls to the underlying executor, then that would unexpectedly provide a way to shut down the executor.

This is definitely a wart, but it's probably our best option, given that default methods mess a little bit with our original assumptions about interfaces. (And it's another weird thing about the Forwarding* classes in general.)

From your linked PR (thanks!), I gather that you filed this issue at least partially because of a failing test (which maybe could be changed to ignore default methods... if not for the exception I mentioned above) but also partially because you're asking whether it's better for forward in this case or not. Let me know if there's other context that I missed from skimming over it.

In any case, we should document this behavior, as we did for ForwardingCollection and friends way back when.

[findepi]

Thanks @cpovirk for your detailed explanation. It makes total sense.

From your linked PR (thanks!), I gather that you filed this issue at least partially because of a failing test (which maybe could be changed to ignore default methods... if not for the exception I mentioned above)

Yes, the test found out the issue, but at least on our side the fix is simple -- we expect a certain class to implement all interface methods explicitly, so we specifically disallow inheriting default methods. It's not a generic Forwarding* class (where we would have concerns like above), it's a concrete class that just happens to be built on Forwarding* class as a foundation.

In any case, we should document this behavior, as we did for ForwardingCollection and friends way back when.

I am sure I have seen this warning there at some point, but forgot about it.
Would it make sense to add some general comment about default methods on the project wiki page?

For ForwardingExecutorService, please consider taking a look at #6299

[cpovirk]

Thanks, we'll get #6299 merged.

As for the wiki, it's mostly been focused on information for end users, rather than for information about the design of Guava. Now, to be fair, the question of how our forwarding classes behave is of interest to end users :) But I think users are significantly more likely to find it in the Javadoc than on the wiki, so I think it falls to us to notice things like this and document them. (I would imagine we could write a test, similar to the one your project has, to notify us of new default methods. I'm not sure even that will be a priority unless we start hearing more reports like this, but it would probably be my next step.)

That still leaves a further question, which is whether it would be good for us to share more of the internal design notes that we have with the public. It's one of those things that we periodically talk about doing. It might still happen one of these days....