0

I have a method that runs every 5 minutes and removes files from a cache

private HashMap<String, CachedFile> cache = new HashMap<>();

@Scheduled(fixedDelay = 300000)
public void deleteFileCache() {

    //remove files last accessed > 12h
    cache.entrySet().stream().filter(entry -> LocalDateTime.now().minus(12, ChronoUnit.HOURS)
            .isAfter(entry.getValue().getLastAccessed())).forEach(entry -> {
        File file = new File(tempFolder, entry.getKey());
        boolean deleted = file.delete();
    });

    //remove entries from HashMap
    cache.entrySet().removeIf(entry -> LocalDateTime.now().minus(12, ChronoUnit.HOURS)
            .isAfter(entry.getValue().getLastAccessed()));

    //if little space left remove oldest files
    long freeSpace = tempFolder.getFreeSpace();
    while (freeSpace < 6000000000L) {
        Optional<String> fileToDelete = cache.entrySet().stream()
                .min(Comparator.comparing(stringCachedFileEntry -> stringCachedFileEntry.getValue().getLastAccessed()))
                .map(Map.Entry::getKey);

        fileToDelete.ifPresent(filename -> {
            new File(tempFolder, filename).delete();
            cache.remove(filename);
        });
        freeSpace = tempFolder.getFreeSpace();
    }
}

This method fails with a ConcurrentModificationException about 2-3 times a day. I am unable to see why since the method can only run once at the same time and I am not iterating at the same time as removing from the HashMap.

It fails at line .min(Comparator.comparing(stringCachedFileEntry ->...

java.util.ConcurrentModificationException: null
        at java.util.HashMap$EntrySpliterator.forEachRemaining(HashMap.java:1704) ~[na:1.8.0_212]
        at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482) ~[na:1.8.0_212]
        at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472) ~[na:1.8.0_212]
        at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708) ~[na:1.8.0_212]
        at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[na:1.8.0_212]
        at java.util.stream.ReferencePipeline.reduce(ReferencePipeline.java:479) ~[na:1.8.0_212]
        at java.util.stream.ReferencePipeline.min(ReferencePipeline.java:520) ~[na:1.8.0_212]
        at ch.my.app.server.FileCacheService.deleteFileCache(FileCacheService.java:113) ~[classes!/:1.0-SNAPSHOT]
7
  • Could you post the full stacktrace? This sounds interesting. Commented Oct 9, 2019 at 14:20
  • 2
    Is there another thread appending to this map? Commented Oct 9, 2019 at 14:21
  • As an aside you would probably want to write 6_000_000_000L for readability. Commented Oct 9, 2019 at 14:22
  • 2
    @isADon It is difficult to say why that line in particular. I recommend that you switch your implementation to a ConcurrentHashMap and see if this yields better results. Any time you use a non-synchronized object with multiple threads, you will run into these kinds of problems. Commented Oct 9, 2019 at 14:31
  • 1
    @isADon: in the presence of concurrent modification (as you said, other threads might add elements while this is running) a HashMap may produce basically any weird behaviour possible. It will try to throw a ConcurrentModificationException, but that's your best case. Use a ConcurrentHashMap or guard any possible modification with the appropriate tools (synchronized blocks or Locks). Commented Oct 9, 2019 at 14:33

1 Answer 1

4

A ConcurrentModificationException is not only thrown by deletions from the iterated set while iterating. Insertions cause it quite as well. So the likely reason for yours is that your Spring boot controller writes to the map without synchronization with the deleteFileCache method.

The obvious solution is to use some thread safe map instead of HashMap, for example the ConcurrentHashMap mentioned in a couple of the comments.

For example, the documentation of Spliterator says:

… After binding a Spliterator should, on a best-effort basis, throw ConcurrentModificationException if structural interference is detected. …

It seems from your stacktrace that the min method uses a Spliterator.

Structural changes include both insertions and deletions (they probably don’t include replacing a mapping in the map where only the value is changed).

Link: Documentation of Spliterator

Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.