AggregatedTestExecutor (and other executors) do not properly close QueryExecutionFactory instances

[abrokenjester]

Expected Behavior

org.aksw.jena_sparql_api.core.QueryExecutionFactory implements AutoCloseable. Any code that uses an instance of this class should take care that the close() method is called so that internal resources can be cleaned up. This can be achieved by either manually calling close() or by making the creation part of a try-with-resources construct.

Actual Behavior

In several places in AggregatedTestExecutor and other executors, Testsource.getExecutionFactory is called on the fly. It is not part of a try-with-resources construct. The result is that the close() method is never called on these instances. For implementations of QueryExecutionFactory that have internal resources (such as open database connections), this is problematic.

[jimkont]

Thanks for the report @jeenbroekstra

I make TestSource AutoClosable which closes any open QueryExecutionFactory objects. When RDFUnit is used as a library the developer would be responsible for closing this (or instantiating it with try-with-resources).

Does this approach cover your use cases? if not, you can reopen the issue and we can find another solution

[abrokenjester]

Thanks for looking into this @jimkont. I think your fixes cover some cases, but there are a few other places where autocloseables are leaking - mainly because they are being daisy-chained in the try block instead of independently assigned. Let me put a PR together for you.

[abrokenjester]

Ah, hang on, I missed the bit where you fixed the close method in AbstractTestSource. Spoke too soon, this might cover our case (though part of the problem is that we use a custom implementation of TestSource that does not inherit from AbstractTestSource).

[jimkont]

Thanks! Any contribution/PR of of course more than welcome!

Would also be happy to refactor TestSource / AbstractTestSource if it results in a design that covers more use cases