fix memory leaks

[DimitrisStaratzis]

Memory leaks

The resource management in the Java API is inconsistent and in some cases non-existent. I have used the Instruments application from Xcode to detect memory leaks. These memory leaks occurred in various ways which I explain with examples below.

Leak kind 1 - Example 1 (Incomplete close())

In the Domain constructor we use ctx.handleError(tiledb.tiledb_domain_alloc(ctx.getCtxp(), _domainpp));
This _domainpp has been created like this:
SWIGTYPE_p_p_tiledb_domain_t _domainpp = tiledb.new_tiledb_domain_tpp();

The problem is that even though the Domain object is AutoClosable and the close() method is being called correctly, we were missing this delete call:
tiledb.delete_tiledb_domain_tpp(_domainpp);

We were only calling tiledb.tiledb_domain_free(domainpp); which in my mind should delete the pointer but it did not. This practice is used throughout the Java API and was present long before me so I can not be sure why it was implemented this way.

Leak kind 2 - Example 2 (Leaking pointer)

Previous implementation:

  public int getAllowDups() throws TileDBError {
    SWIGTYPE_p_int allowsDupsPtr = tiledb.new_intp();
    try {
      ctx.handleError(
          tiledb.tiledb_array_schema_get_allows_dups(ctx.getCtxp(), getSchemap(), allowsDupsPtr));

      return tiledb.intp_value(allowsDupsPtr);
    } catch (TileDBError err) {
      tiledb.delete_intp(allowsDupsPtr);
      throw err;
    }
  }

The SWIGTYPE_p_int allowsDupsPtr above is leaking. Methods of this type have been changed to:

  public int getAllowDups() throws TileDBError {
    SWIGTYPE_p_int allowsDupsPtr = tiledb.new_intp();
    try {
      ctx.handleError(
          tiledb.tiledb_array_schema_get_allows_dups(ctx.getCtxp(), getSchemap(), allowsDupsPtr));

      return tiledb.intp_value(allowsDupsPtr);
    } finally {
      tiledb.delete_intp(allowsDupsPtr);
    }
  }

The finally keyword is ideal for resource management. It runs regardless of the execution success.

Leak kind 3 - Example 3 (Internal use of TileDB objects)

This leak refers to cases where the API was using its own objects but was not releasing its resources after use.

  public synchronized SubArray addRangeByName(String name, Object start, Object end, Object stride)
      throws TileDBError {
    Datatype dimType;
    try (ArraySchema schema = array.getSchema();
        Domain domain = schema.getDomain()) {
      dimType = domain.getDimension(name).getType();
    }

In the example above, getDimension() returns a new Dimension() object which is never closed(). These kind of methods have been changed to:

  public synchronized SubArray addRangeByName(String name, Object start, Object end, Object stride)
      throws TileDBError {
    Datatype dimType;
    try (ArraySchema schema = array.getSchema();
        Domain domain = schema.getDomain();
        Dimension dim = domain.getDimension(name)) {
      dimType = dim.getType();
    }

Leak kind 4

Some Classes were not implementing the AutoClosable interface. (FragmentInfo, DimensionLabel)

Extra

Finally I have removed all try{}catch(){} blocks from the setters across the API as they were useless. ctx.handleError() throws errors with their messages. This is not relevant to the leaks but adds consistency in the code. Example:

before:

  public void setFillValue(NativeArray value, BigInteger size) throws TileDBError {
    Util.checkBigIntegerRange(size);
    try {
      ctx.handleError(
          tiledb.tiledb_attribute_set_fill_value(
              ctx.getCtxp(), attributep, value.toVoidPointer(), size));
    } catch (TileDBError err) {
      throw err;
    }
  }

after:

  public void setFillValue(NativeArray value, BigInteger size) throws TileDBError {
    Util.checkBigIntegerRange(size);
    ctx.handleError(
        tiledb.tiledb_attribute_set_fill_value(
            ctx.getCtxp(), attributep, value.toVoidPointer(), size));
  }

[ihnorton]

LGTM, thanks. Please file a follow-up item to add a leak test similar to what we do in tiledb-py.

[ihnorton]

Oh, also - at some point we should talk about whether we can reduce the boilerplate in the manually-written api functions using some generic wrappers (I don't know enough about java generics yet to know if it is possible, though).

[DimitrisStaratzis]

in tiledb-py

https://app.shortcut.com/tiledb-inc/story/36059/add-memory-leak-check-in-the-java-api