[BUG]关于SpringValueRegistry.scanAndClean方法报并发修改异常的疑问

[lzbjut]

• I have checked the discussions
• I have searched the issues of this repository and believe that this is not a duplicate.
• I have checked the FAQ of this repository and believe that this is not a duplicate.

Describe the bug
最近使用旧版apollo时遇到prototype-bean导致内存被打满的问题，查找issue找到#1670
在查看相关方法时，找到了遇到并发问题并解决的：#2281
但是查看SpringValueRegistry类的issue还是发现了：#4355、#4356、#4800、#4925、#5169
问题主要集中于scanAndClean还是有报并发错误，我查看synchronizedListMultimap方法的注释时，发现有相关叙述：

  /**
   * Returns a synchronized (thread-safe) multimap backed by the specified multimap. In order to
   * guarantee serial access, it is critical that <b>all</b> access to the backing multimap is
   * accomplished through the returned multimap.
   *
   * <p>It is imperative that the user manually synchronize on the returned multimap when accessing
   * any of its collection views:
   *
   * <pre>{@code
   * Multimap<K, V> multimap = Multimaps.synchronizedMultimap(
   *     HashMultimap.<K, V>create());
   * ...
   * Collection<V> values = multimap.get(key);  // Needn't be in synchronized block
   * ...
   * synchronized (multimap) {  // Synchronizing on multimap, not values!
   *   Iterator<V> i = values.iterator(); // Must be in synchronized block
   *   while (i.hasNext()) {
   *     foo(i.next());
   *   }
   * }
   * }</pre>
   *
   * <p>Failure to follow this advice may result in non-deterministic behavior.
   *
   * <p>Note that the generated multimap's {@link Multimap#removeAll} and {@link
   * Multimap#replaceValues} methods return collections that aren't synchronized.
   *
   * <p>The returned multimap will be serializable if the specified multimap is serializable.
   *
   * @param multimap the multimap to be wrapped in a synchronized view
   * @return a synchronized view of the specified multimap
   */

其中It is imperative that the user manually synchronize on the returned multimap when accessing any of its collection views:
Failure to follow this advice may result in non-deterministic behavior.
是否说明synchronizedMultimap的iterator实际上是非线程安全的，于是我就尝试自己写了一个简单的复现代码，并获得了其他issue中提到的错误
To Reproduce
复现这个synchronizedMultimap并发写入并定期清理导致报错的场景，我复现出其中一种，即输入固定量的k-v，定时清除，在写入停止后，因为剩余值(registry.entries().size())为负,在获取iterator时会报错：negative size

    @Test
    public void testConfig() {
        Multimap<String, String> registry = Multimaps.synchronizedListMultimap(LinkedListMultimap.create());
        ExecutorService executorService = Executors.newFixedThreadPool(10);
        ScheduledExecutorService scheduledExecutorService = Executors.newSingleThreadScheduledExecutor();
        AtomicInteger removeTotal = new AtomicInteger();
        for(int i=0;i<10;i++){
            executorService.submit(()->{
                try {
                    for(int j = 0;j<100000;j++){
                        Thread.sleep(1);
                        registry.put("aa",System.currentTimeMillis()+"");
                    }
                    System.out.println("done");

                }catch (Exception e){
                    e.printStackTrace();
                }

            });
        }
        scheduledExecutorService.scheduleWithFixedDelay(()->{
                try {
                    long start = System.currentTimeMillis();
                        Iterator<Map.Entry<String, String>> ss = registry.entries().iterator();
                        while (ss.hasNext()) {
                                Map.Entry<String, String> entry = ss.next();
                                if (!StringUtils.isNullOrEmpty(entry.getValue())) {
                                    ss.remove();
                                    removeTotal.incrementAndGet();
                                }
                        }
                    System.out.println("cost:"+(System.currentTimeMillis()-start)+"ms");
                    System.out.println("removed total: "+removeTotal.get());
                    System.out.println("remain: "+registry.entries().size());

                }catch (Exception e){
                    System.out.println(e.getMessage());
                    e.printStackTrace();
                }

            },5,5,TimeUnit.SECONDS);
        try {
            Thread.sleep(1000000000);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }

Additional Details & Logs

当我为清理代码增加synchronized后该问题解决：

    synchronized (registry) {
        Iterator<Map.Entry<String, String>> ss = registry.entries().iterator();
        while (ss.hasNext()) {
            Map.Entry<String, String> entry = ss.next();
            if (!StringUtils.isNullOrEmpty(entry.getValue())) {
                ss.remove();
                removeTotal.incrementAndGet();
            }
        }
    }

[dosubot]

Hey, @lzbjut! I'm here to help you with any bugs, questions, or contributions you have in mind. Let's work together to resolve any issues you're facing.

The issue you're encountering with the scanAndClean method in the SpringValueRegistry class seems to be related to the use of synchronizedListMultimap. As you correctly pointed out, the documentation for synchronizedMultimap emphasizes the need to manually synchronize on the multimap when accessing any of its collection views. This is because the iterator obtained from a synchronized multimap is not inherently thread-safe, which can lead to concurrency issues like the ones you're experiencing.

In your test case, you managed to reproduce the concurrency issue by performing concurrent writes and periodic cleanups on the multimap. The error you encountered, such as a "negative size" exception, is indicative of concurrent modifications without proper synchronization.

To resolve this, you added a synchronized block around the cleanup code, which is the correct approach. By synchronizing on the multimap, you ensure that the iterator is accessed in a thread-safe manner, preventing concurrent modification exceptions.

This pattern of manually synchronizing access to collection views is crucial when using synchronized multimaps in a concurrent environment. It ensures that all operations on the multimap are performed atomically, maintaining data consistency and preventing runtime errors.

To continue talking to Dosu, mention @dosu.

---

^{Help Dosu learn! Give it feedback: Great Response | Irrelevant Answer | Incorrect Sources | Too Verbose | Hallucination | Other}

[nobodyiam]

非常感谢，看来找到根因了