CODEWITHSUNDEEP
javamultithreadingconcurrency
user1599840
user1599840

Reputation: 75

Implementation of cache expiration with Future

I'm trying to extend the cache implementation given in book Concurrency in Practice(by Brian Goetz).

I want to extend the cache such that if any entry is not accessed in 10 seconds then it should expire. i.e removed from cache.

For this I have extended Future and FutureTask as below

package examples.cache;

import java.util.concurrent.Future;

public interface FutureWithExpire<V> extends Future<V> {
    public boolean isResultExpired();
}

And

package examples.cache;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.FutureTask;

public class FutureTaskWithExpire<V> extends FutureTask<V> implements FutureWithExpire<V>{
    private final static long validPeriod = 10 * 1000;
    private long expirationTime;

    public FutureTaskWithExpire(Callable<V> callable) {
        super(callable);
    }

    @Override
    public V get() throws InterruptedException,ExecutionException{
        V v = super.get();
        expirationTime = System.currentTimeMillis() + validPeriod;
        return v;
    }

    public boolean isResultExpired(){
    if(isDone()){
        if(expirationTime < System.currentTimeMillis()){
        return true;
        }
    }
    return false;
    }
}

As seen above I'm overriding the the get method to calculate the expiration time.

And in the main cache Class i.e Memorizer I will start a new thread that will periodically scan the entries and remove entries where isResultExpired returns true.

I just wanted to know if this implementation would work or there is some bug in my code?

Note I should also override get with Timeout. However, to keep it concise I have omitted that.

Upvotes: 4

Views: 1286

Answers (2)

user1599840
user1599840

Reputation: 75

And Ofcourse expirationTime should be volatile

Upvotes: 1

Eugene
Eugene

Reputation: 120978

Why is this code: 

 expirationTime = System.currentTimeMillis() + validPeriod;

in the get method? Seems to me it should be in the constructor, otherwise it could be invalidated way too fast. Suppose that you created the FutureTaskWithExpire instance and some other Thread, the one that is suppose to invalidate entries calls the isResultExpired method, expiration time will be actually zero and thus the entry will be invalidated, even though it still had time live.

Also I think that the calls to get and isExpired should be synchronized on a ReentrantLock for example, because you do not want some Thread to invalidate your entry, while in another Thread you are trying to give that entry.

Upvotes: 0

Related Questions