CacheClient.close() not adhering to Closeable interface

[galderz]

CacheEntryListenerClient extends CacheClient which extends Closeable.

Closeable defines close() as:

public void close() throws IOException;

But CacheClient forgets about throwing IOException when it overrides close(). Moreover, CacheClient.close() includes calls to:

client.invoke(Server.CLOSE_OPERATION);

Which can throw RuntimeException. As a result, it can happen that when closing the listener, a RuntimeException can be thrown which ends up bubbling up, e.g.

[ERROR] Tests run: 14, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.258 s <<< FAILURE! - in org.jsr107.tck.event.CacheListenerTest
[ERROR] testCacheClearListener[true](org.jsr107.tck.event.CacheListenerTest)  Time elapsed: 0.016 s  <<< ERROR!
java.lang.RuntimeException: Failed to perform operation CLOSE
	at org.jsr107.tck.event.CacheListenerTest.onAfterEachTest(CacheListenerTest.java:144)
Caused by: java.net.SocketException: Broken pipe (Write failed)
	at org.jsr107.tck.event.CacheListenerTest.onAfterEachTest(CacheListenerTest.java:144)

CacheClient.close() should play nicely and whatever error it gets, throw it as a IOException.

[cruftex]

Galder,

thanks for reaching out. It takes me always an hour to know again, what is going on here.

The TCK uses a client/server communication model for the customizations (e.g. CacheLoader, etc.).
This is because these may be created inside a different VM or even on a different machine, in case of an distributed cache. The server is run by the TCK, the client part is created by the cache implementation.

The client part, as you discovered, has no mechanism to deal with exceptions when close() is called. I did discover the same when reviewing and extending that code. But, there is no TCK test for exceptions on close (so far I have it in mind). The client part wraps the actual customization code from the TCK. So, there is something missing, but it is actually not needed.

Something looks wrong. May be the error handling in the TCK can be improved. But I don't see clearly where the problem is, at the moment. What you are experiencing might be a consequence of the change in #100

Can you please check, and maybe give more information how to reproduce the problem (TCK version, cache implementation, etc....)?

[cruftex]

Since there is no update, I'll close this issue. As described the requested change in CacheClient.close() is not necessary. In case there is more information please feel free to open a new issue.

[galderz]

I'm sorry but if feel it's necessary. I'm having to deal with Throwable/Runtime exceptions because of misbehaving TCK classes which do not want to stick to the contract which says: throws IOException

[galderz]

I want this issue reopened to allow more time to discuss it.

[cruftex]

I want this issue reopened to allow more time to discuss it.

No problem.

I'm sorry but if feel it's necessary. I'm having to deal with Throwable/Runtime exceptions because of misbehaving TCK classes which do not want to stick to the contract which says: throws IOException

Please check for the root cause of your exceptions or provide steps to reproduce.

[galderz]

I've sent a PR that fixes the TCK code. Thanks

[cruftex]

Thanks for the PR. In the PR you now wrap all exceptions to IOExceptions.

1. There is no interface contract on Closeable, which says, that only IOExceptions are allowed.
   Wrapping arbitrary RuntimeExceptions is not needed and contrary to the intention of the interface, which says: "@throws IOException if an I/O error occurs". A RuntimeException needs to be properly handled as well by any interface user.

2. If there is any exception on close, something is odd at another place. I cannot see how the PR relates to the problems you are experiencing.

[cruftex]

Another two remarks:

Maybe your code seems to work because the tested implementations does ignore IOExceptions. But this doesn't necessarily mean that there are no problems.

JCache/JSR107 defines that customizations may implement Closeable. Since the Spec targets JDK 1.6 Closeable is the only choice for it. Since Java 7 we would probably use AutoCloseable, which allows every kind of checked exception as well.