仅使用JDK(6)提供类来改进代码的方法？(并发,线程安全)

Question

(初步说明:也许这更适合代码审查？)

编辑 回答自己 ; 我相信这个答案涵盖了我的所有需求/问题,当然,欢迎提出意见.原始问题留待下面参考.

你好,

这里感兴趣的是.getSources()方法.此方法旨在返回给定的消息源列表Locale.

此方法的两个中央数据结构是sources和failedLookups,请参阅注释代码.

这个特定的实现.getSources()只能返回一个空列表或单个元素列表,具体取决于tryAndLookup()原型的方法:

protected abstract MessageSource tryAndLookup(final Locale locale)
    throws IOException;

现在,代码的逻辑如下:

• 如果已成功查找该语言环境的消息源,则返回该消息来源;
• 从这一点开始,没有进行任何查找; 但是,这是否意味着这是否意味着先前的查找尝试已经完成:检查失败的查找集,如果要查找的语言环境在那里,则是已知失败,返回空列表;
• 现在,已知的情况是这个区域设置查找实际上从未执行过:执行它; 根据tryAndLookup方法的结果,记录成功或失败.

现在,为什么我会这么做:我无法控制tryAndLookup(); 在返回有效来源或失败之前,可能需要花费过多的时间才能执行.结果,我不愿意使用粗锁或synchronized块.

/**
 * Set of locales known to have failed lookup.
 *
 * <p>When a locale is in this set, it will not attempt to be reloaded.</p>
 */
private final Set<Locale> lookupFailures
    = new CopyOnWriteArraySet<Locale>();

/**
 * Set of message sources successfully looked up
 *
 * <p>When a source is in there, it is there permanently for now.</p>
 */
private final ConcurrentMap<Locale, MessageSource> sources
    = new ConcurrentHashMap<Locale, MessageSource>();

@Override
protected final List<MessageSource> getSources(final Locale locale)
{
    MessageSource source = sources.get(locale);

    /*
     * If found, return it
     */
    if (source != null)
        return Arrays.asList(source);

    /*
     * If it is a registered failure, return the empty list
     */
    if (lookupFailures.contains(locale))
        return Collections.emptyList();

    /*
     * OK, try and look it up. On success, register it in the sources map.
     * On failure, record the failure an return the empty list.
     */
    try {
        source = tryAndLookup(locale);
        sources.putIfAbsent(locale, source);
        // EDIT: fix for bug pinpointed by JBNizet
        // was:
        // return Arrays.asList(source);
        // now is:
        return Arrays.asList(sources.get(locale));
    } catch (IOException ignored) {
        lookupFailures.add(locale);
        return Collections.emptyList();
    }
}

我的问题有三个:

• 我故意将自己限制在仅适用于JDK的类中; 我选择ConcurrentHashMap作为ConcurrentMap实现,并CopyOnWriteArraySet作为线程安全的Set实现; 来自javadoc,这些是我能找到的最好的.但是我被某个地方误导了吗？
• 我认为这段代码最终是线程安全的; 例如,某些极端情况可能会导致查找不止一次,但这就是我这样做的原因.putIfAbsent(); 现在我一直使用和信任Guava LoadingCache用于缓存目的,这是我第一次涉足这个领域; 这段代码实际上是线程安全吗？
• 这段代码有一个致命的缺陷:多个线程可能同时执行tryAndLookup()...有什么解决方案让这个方法每次查找只执行一次？

Answer 1

回答自己...

该算法已被彻底修改。它基于 JDK 的FutureTask，因为它有两个非常好的属性：

• 它的.run()方法是异步的；
• 它是有状态的，无论成功还是失败——这意味着，它将返回已经计算的结果或抛出的异常（包装到ExecutionException）.get()。

这对所使用的数据结构有很大的影响：

• 先前存在的lookupFailures记录失败的集合已经消失；
• 现有的地图，原来是 a ConcurrentMap，现在被替换为平原Map，并由 a 守卫ReentrantLock；它的值现在FutureTask<MessageSource>代替了MessageSource。

这对算法也有很大的影响，算法要简单得多：

• 锁定地图；
• 查找区域设置的任务；如果它以前不存在，则创建它，然后.run()它；
• 解锁地图；
• .get()结果：成功时返回单个元素列表，失败时返回空列表。

完整代码，带注释：

@ThreadSafe
public abstract class CachedI18NMessageBundle
    extends I18NMessageBundle
{
    /**
     * Map pairing locales with {@link FutureTask} instances returning message
     * sources
     *
     * <p>There will only ever be one task associated with one locale; we
     * therefore choose to make it a normal map, guarded by a {@link
     * ReentrantLock}.</p>
     *
     * <p>The tasks' {@link FutureTask#run()} method will be executed the first
     * time this object is initialized.</p>
     */
    @GuardedBy("lock")
    private final Map<Locale, FutureTask<MessageSource>> lookups
        = new HashMap<Locale, FutureTask<MessageSource>>();

    /**
     * Lock used to guarantee exclusive access to the {@link #lookups} map
     */
    private final Lock lock = new ReentrantLock();

    @Override
    protected final List<MessageSource> getSources(final Locale locale)
    {
        FutureTask<MessageSource> task;

        /*
         * Grab an exclusive lock to the lookups map. The lock is held only for
         * the time necessary to grab the FutureTask or create it (and run it)
         * if it didn't exist previously.
         *
         * We can do this, since FutureTask's .run() is asynchronous.
         */
        lock.lock();
        try {
            /*
             * Try and see whether there is already a FutureTask associated with
             * this locale.
             */
            task = lookups.get(locale);
            if (task == null) {
                /*
                 * If not, create it and run it.
                 */
                task = lookupTask(locale);
                lookups.put(locale, task);
                task.run();
            }
        } finally {
            lock.unlock();
        }

        /*
         * Try and get the result for this locale; on any failure event (either
         * an IOException thrown by tryAndLookup() or a thread interrupt),
         * return an empty list.
         */
        try {
            return Arrays.asList(task.get());
        } catch (ExecutionException ignored) {
            return Collections.emptyList();
        } catch (InterruptedException  ignored) {
            return Collections.emptyList();
        }
    }

    protected abstract MessageSource tryAndLookup(final Locale locale)
        throws IOException;

    @Override
    public final Builder modify()
    {
        throw new IllegalStateException("cached bundles cannot be modified");
    }

    /**
     * Wraps an invocation of {@link #tryAndLookup(Locale)} into a {@link
     * FutureTask}
     *
     * @param locale the locale to pass as an argument to {@link
     * #tryAndLookup(Locale)}
     * @return a {@link FutureTask}
     */
    private FutureTask<MessageSource> lookupTask(final Locale locale)
    {
        final Callable<MessageSource> callable = new Callable<MessageSource>()
        {
            @Override
            public MessageSource call()
                throws IOException
            {
                return tryAndLookup(locale);
            }
        };

        return new FutureTask<MessageSource>(callable);
    }
}

我什至可以在查找成功和失败时测试“创建的单个任务”，因为它tryAndLookup()是抽象的，因此可以使用 Mockito 进行“监视”。完整的测试类源代码在这里（方法onlyOneTaskIsCreatedPer*LocaleLookup()）。