commons-collections: CircularFifoQueue 线程非安全

1. 代码分析

commons集合

<dependency>
    <groupId>commons-collections</groupId>
    <artifactId>commons-collections</artifactId>
    <version>4.4</version>
</dependency>

CircularFifoQueue.java

/**
 * CircularFifoQueue is a first-in first-out queue with a fixed size that
 * replaces its oldest element if full.
 * <p>
 * The removal order of a {@link CircularFifoQueue} is based on the
 * insertion order; elements are removed in the same order in which they
 * were added.  The iteration order is the same as the removal order.
 * </p>
 * <p>
 * The {@link #add(Object)}, {@link #remove()}, {@link #peek()}, {@link #poll},
 * {@link #offer(Object)} operations all perform in constant time.
 * All other operations perform in linear time or worse.
 * </p>
 * <p>
 * This queue prevents null objects from being added.
 * </p>
 *
 * @param <E> the type of elements in this collection
 * @since 4.0
 */
public class CircularFifoQueue<E> extends AbstractCollection<E>
    implements Queue<E>, BoundedCollection<E>, Serializable {

    /** Serialization version. */
    private static final long serialVersionUID = -8423413834657610406L;

    /** Underlying storage array. */
    private transient E[] elements;

    /** Array index of first (oldest) queue element. */
    private transient int start = 0;

    /**
     * Index mod maxElements of the array position following the last queue
     * element.  Queue elements start at elements[start] and "wrap around"
     * elements[maxElements-1], ending at elements[decrement(end)].
     * For example, elements = {c,a,b}, start=1, end=1 corresponds to
     * the queue [a,b,c].
     */
    private transient int end = 0;

    /** Flag to indicate if the queue is currently full. */
    private transient boolean full = false;

    /** Capacity of the queue. */
    private final int maxElements;

    /**
     * Constructor that creates a queue with the default size of 32.
     */
    public CircularFifoQueue() {
        this(32);
    }

    /**
     * Constructor that creates a queue with the specified size.
     *
     * @param size  the size of the queue (cannot be changed)
     * @throws IllegalArgumentException  if the size is &lt; 1
     */
    @SuppressWarnings("unchecked")
    public CircularFifoQueue(final int size) {
        if (size <= 0) {
            throw new IllegalArgumentException("The size must be greater than 0");
        }
        elements = (E[]) new Object[size];
        maxElements = elements.length;
    }

    /**
     * Adds the given element to this queue. If the queue is full, the least recently added
     * element is discarded so that a new element can be inserted.
     *
     * @param element  the element to add
     * @return true, always
     * @throws NullPointerException  if the given element is null
     */
    @Override
    public boolean add(final E element) {
        if (null == element) {
            throw new NullPointerException("Attempted to add null object to queue");
        }

        if (isAtFullCapacity()) {
            remove();
        }

        elements[end++] = element;

        if (end >= maxElements) {
            end = 0;
        }

        if (end == start) {
            full = true;
        }

        return true;
    }

内部实现是 elements 数组，在构造函数中就初始化好了；

add方法没有synchronized同步关键字，对elements的访问也不在同步块中“elements[end++] = element;” ，是线程非安全的【代码注释文档中没看到具体的描述】

TODO end的类型是transient

报错信息：（一个多月才能跑出来）

2.测试

CircularFifoQueue虽然是线程非安全的，但是不好复现，使用如下的测试代码，在8C16G物理机上跑30m也没有跑出来问题；

分析代码：

    /** Underlying storage array. */
    private transient E[] elements;
    
    elements[end++] = element;

time	thread-1	thread-2
1	end=98 end++      返回值99，end=99
2		end=99 end++      返回值100，end=100
3		elements[100]  报错
4	elements[100]  报错

end++ 和 elements[] ，恰好卡在线程切换间隙，是不是造成了不太好验证？

import org.apache.commons.collections4.queue.CircularFifoQueue;
import org.apache.commons.lang3.RandomStringUtils;

import java.util.concurrent.Executors;
import java.util.concurrent.ThreadPoolExecutor;

public class CircularFifoQueueTest {

    public static void main(String[] args) {
        multiThreadTest();
    }

    public static void multiThreadTest() {
        CircularFifoQueue<String> queue = new CircularFifoQueue(10);
        final int threadNum = 100;
        ThreadPoolExecutor executor = (ThreadPoolExecutor) Executors.newFixedThreadPool(threadNum);
        for (int i = 1; i < threadNum; i++) {
            executor.submit(() -> {
                while (true) {
                    add(queue);
                    // compare(queue);
                }
            });
        }
        Utils.sleep(10000);
    }

    public static void add(CircularFifoQueue<String> queue) {
        for (int i = 1; i < 100; i++) {
            String str = RandomStringUtils.randomAlphanumeric(1, 100);
            System.out.println("thread:" + Thread.currentThread().getName() + " add str:" + str);
            queue.add(str);
        }
    }

    public static void compare(CircularFifoQueue<String> queue) {
        for (String str : queue) {
            System.out.println("thread:" + Thread.currentThread().getName() + " compare str:" + str);
            if ("test".equalsIgnoreCase(str)) {
                System.out.println("thread:" + Thread.currentThread().getName() + " found...");
                break;
            }
        }
    }

}