typetools / typetools/checker-framework

Unexpected `required.method.not.called` when implementing a resources collection/map

Open
#6,054 11 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

ResourceLeakChecker
Dominant language
Java
Stars
1.1k
Forks
440
Avg merge
1d 12h
Merged PRs (30d)
134

Description

So I was trying to implement a resources map to be used to such situations, I have this:

@InheritableMustCall({ "removeAll" })
public class ResourcesMap<K, V>
{
    private final Map<K, V> map;

    public ResourcesMap()
    {
        this.map = new HashMap<>();
    }

    public @MustCallAlias V get(K key)
    {
        return map.get(key);
    }

    public @Owning V remove(K key)
    {
        return map.remove(key);
    }

    @CreatesMustCallFor
    public @MustCallAlias V put(K key, @Owning V value)
    {
        return map.put(key, value);
    }

    public void forEach(Consumer<@MustCallAlias V> consumer)
    {
        map.values().forEach(consumer);
    }

    public <T> void removeAll(T initial, OwningBiFunction<T, V> consumer)
    {
        Iterator<V> iterator = map.values().iterator();
        T acc = initial;
        while (iterator.hasNext())
        {
            V value = iterator.next();
            try
            {
                iterator.remove();
            }
            finally
            {
                acc = consumer.apply(acc, value);
            }
        }
    }

    public <T> void removeAll(OwningConsumer<V> consumer)
    {
        Iterator<V> iterator = map.values().iterator();
        while (iterator.hasNext())
        {
            V value = iterator.next();
            try
            {
                iterator.remove();
            }
            finally
            {
                consumer.accept(value);
            }
        }
    }

    public interface OwningConsumer<V> extends Consumer<V>
    {
        @Override
        void accept(@Owning V v);
    }

    public interface OwningBiFunction<T, V> extends BiFunction<T, V, T>
    {
        @Override
        T apply(T t, @Owning V v);
    }
}

which is used in the following way:

        ResourcesMap<TableId, UncommittedTableData.FlushWriter> flushWriters = new ResourcesMap<>();
        try (CloseableIterator<PaxosKeyState> iterator = updateSupplier.flushIterator(paxos))
        {
            while (iterator.hasNext())
            {
                PaxosKeyState next = iterator.next();
                UncommittedTableData.FlushWriter writer = flushWriters.get(next.tableId);
                if (writer == null)
                {
                    writer = getOrCreateTableState(next.tableId).flushWriter();
                    flushWriters.put(next.tableId, writer);
                }
                writer.append(next);
            }

            flushWriters.removeAll(UncommittedTableData.FlushWriter::finish);
        }
        catch (IOException t)
        {
            flushWriters.removeAll((Throwable) t, (acc, flushWriter) -> flushWriter.abort((acc)));
            throw new IOException(t);
        }

where the FlushWriter is something like:

    @InheritableMustCall({ "finish", "abort" })
    public interface FlushWriter
    {
        void append(PaxosKeyState commitState) throws IOException;

        void finish();

        Throwable abort(Throwable accumulate);

        default void appendAll(Iterable<PaxosKeyState> states) throws IOException
        {
            for (PaxosKeyState state : states)
                append(state);
        }
    }

Now, when I try to check it, it complains with the following:

    [javac] /home/jlewandowski/dev/cassandra/c18190-ecj/src/java/org/apache/cassandra/service/paxos/uncommitted/PaxosUncommittedTracker.java:161: error: [builder:required.method.not.called] @MustCall methods removeAll, finish, abort may not have been invoked on flushWriters or any of its aliases.
    [javac]         ResourcesMap<TableId, UncommittedTableData.FlushWriter> flushWriters = new ResourcesMap<>();
    [javac]                                                                 ^
    [javac]   The type of object is: org.apache.cassandra.utils.ResourcesMap<org.apache.cassandra.schema.TableId,org.apache.cassandra.service.paxos.uncommitted.UncommittedTableData.FlushWriter>.
    [javac]   Reason for going out of scope: possible exceptional exit due to throw new IOException(t); with exception type java.io.IOException

I'm not sure what I'm doing wrong there?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the diagnostic at PaxosUncommittedTracker.java:161 and the ResourcesMap/FlushWriter must-call annotations; inspect how the checker tracks aliases through try/catch exceptional exits and removeAll. Reproduce the report by running the checker on this example, then confirm the valid cleanup path is accepted without losing required-method diagnostics.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.