Где ошибка в этом фрагменте многопоточного кода?

Question

Я создал класс, который используется в качестве поставщика кеша.Он использует карту, записи карты с метками времени и порождает поток, который время от времени выполняет очистку.Этот класс используется в веб-приложении.В этом веб-приложении возникла проблема, когда POST занимал 30 секунд.Я отследил проблему до этого класса кеша, устраняя ее, чтобы решить проблему.

Я изо всех сил старался найти ошибку в этом классе, но не могу.Пожалуйста, помогите мне здесь.Предположим, что класс User - это своего рода POJO, описывающий пользователя.

public class UserStore implements Thread.UncaughtExceptionHandler {
private static volatile UserStore instance;
private static Thread cleanUpThread;
private static Map<String, TimeStampedToken<User>> tokenMap = new HashMap<String, TimeStampedToken<User>>();
public static UserStore getInstance() {
    if (instance == null) {
         synchronized(UserStore.class) {
             if (instance == null) {
                 instance = new UserStore();
                 cleanUpThread = new Thread(new CleanUpWorker());
                 cleanUpThread.setUncaughtExceptionHandler(instance);
                 cleanUpThread.start();
             }
         }
    }
    return instance;
}
public void uncaughtException(Thread thread, Throwable throwable) {
    if (throwable instanceof ThreadDeath) {
         cleanUpThread = new Thread(new CleanUpWorker());
         cleanUpThread.setUncaughtExceptionHandler(this);
         cleanUpThread.start();
         throw (ThreadDeath)throwable;
    }

}
private static class CleanUpWorker implements Runnable {
    private static final long CLEANUP_CYCLE_MS = 300000;
    private static final long OBJECT_LIVE_TIME = 299900;
    public void run() {
        long sleepRemaining; 
        long sleepStart = System.currentTimeMillis();
        sleepRemaining = CLEANUP_CYCLE_MS; 
        while (true) {
            try {
                sleepStart = System.currentTimeMillis();
                Thread.sleep(sleepRemaining);
                cleanUp();
                sleepRemaining = CLEANUP_CYCLE_MS;
            } catch (InterruptedException e) {
                sleepRemaining = System.currentTimeMillis() - sleepStart;
            }
        }
    }
    private void cleanUp() {
        Long currentTime = System.currentTimeMillis();
        synchronized(tokenMap) {
            for (String user : tokenMap.keySet()) {
                TimeStampedToken<User> tok = tokenMap.get(user);
                if (tok.accessed + OBJECT_LIVE_TIME < currentTime) {
                    tokenMap.remove(user);
                }
            }
        }
    }

}
public void addToken(User tok) {
    synchronized(tokenMap) {
        tokenMap.put(tok.getUserId(), new TimeStampedToken<User>(tok));
    }
}
public User getToken(String userId) {
    synchronized(tokenMap) {
        TimeStampedToken<User> user = tokenMap.get(userId);
        if (user != null) {
            user.accessed = System.currentTimeMillis();
            return user.payload;
        } else {
            return null;
        }

    }
}
private static class TimeStampedToken<E> {
    public TimeStampedToken(E payload) {
        this.payload = payload;
    }
    public long accessed = System.currentTimeMillis();
    public E payload; 
}
}

Answer 1

Вот как бы я подошел к этому.В случае многопоточного кода простота часто является наилучшим подходом, поскольку с большей вероятностью он будет работать.

(третий параметр true в LinkedHashMap означает, что итераторы в этой карте следуют за порядком доступа, а не порядкомвставка)

public enum UserStore {
    ;

    interface User {
        String getUserId();
    }

    // a LRU cache with a timestamp.
    private static final Map<String, TimeStampedToken<User>> tokenMap = new LinkedHashMap<String, TimeStampedToken<User>>(16, 0.7f, true);
    private static final long OBJECT_LIVE_TIME = 299900;

    public static synchronized void addToken(User tok) {
        final long now = System.currentTimeMillis();
        // clean up as we go
        for (Iterator<Map.Entry<String, TimeStampedToken<User>>> iter = tokenMap.entrySet().iterator(); iter.hasNext(); ) {
            final Map.Entry<String, TimeStampedToken<User>> next = iter.next();
            if (next.getValue().accessed + OBJECT_LIVE_TIME >= now)
                // the map is ordered by access time so there are no more to clean up.
                break;
            iter.remove();
        }
        // add a new entry
        tokenMap.put(tok.getUserId(), new TimeStampedToken<User>(tok, now));
    }

    public static synchronized User getToken(String userId) {
        final long now = System.currentTimeMillis();
        TimeStampedToken<User> user = tokenMap.get(userId);
        if (user == null)
            return null;

        user.accessed = now;
        return user.payload;
    }

    static class TimeStampedToken<E> {
        long accessed;
        final E payload;

        TimeStampedToken(E payload, long now) {
            this.payload = payload;
            accessed = now;
        }
    }
}

Answer 2

Эта линия выглядит странно для меня ...

sleepRemaining = System.currentTimeMillis () - sleepStart;

... конечно, это должно быть ...

sleepRemaining = CLEANUP_CYCLE_MS - (System.currentTimeMillis () - sleepStart);