From 7c837c1c29c0403be0c367520fa9ed78308125b3 Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Fri, 5 Dec 2014 14:46:30 +0300 Subject: [PATCH] EA-56355 - assert: PrioritizedDocumentListener$.getPriority; fix racey list.toArray(new T[list.size()]) --- .../src/com/intellij/lang/CompositeLanguage.java | 7 +++++-- .../openapi/editor/impl/DocumentImpl.java | 7 ++----- .../vfs/impl/VirtualFilePointerContainerImpl.java | 9 ++++----- .../intellij/psi/impl/PsiDocumentManagerBase.java | 8 +++----- .../impl/config/IntentionManagerImpl.java | 5 +++-- .../openapi/editor/impl/EditorFactoryImpl.java | 3 ++- .../util/src/com/intellij/util/ArrayUtil.java | 15 +++++++++++++++ .../intellij/util/containers/ContainerUtil.java | 6 ++++++ .../application/CvsEntriesManager.java | 6 ++---- 9 files changed, 42 insertions(+), 24 deletions(-) diff --git a/platform/core-api/src/com/intellij/lang/CompositeLanguage.java b/platform/core-api/src/com/intellij/lang/CompositeLanguage.java index 43b942b2e587..244f186b0c24 100644 --- a/platform/core-api/src/com/intellij/lang/CompositeLanguage.java +++ b/platform/core-api/src/com/intellij/lang/CompositeLanguage.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2009 JetBrains s.r.o. + * Copyright 2000-2014 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,7 +17,9 @@ package com.intellij.lang; import com.intellij.psi.PsiFile; +import com.intellij.util.ArrayUtil; import com.intellij.util.containers.ContainerUtil; +import org.jetbrains.annotations.NotNull; import java.util.ArrayList; import java.util.List; @@ -53,7 +55,8 @@ public class CompositeLanguage extends Language { return extensions.toArray(new Language[extensions.size()]); } + @NotNull public LanguageFilter[] getLanguageExtensions() { - return myFilters.toArray(new LanguageFilter[myFilters.size()]); + return ArrayUtil.stripTrailingNulls(myFilters.toArray(new LanguageFilter[myFilters.size()])); } } diff --git a/platform/core-impl/src/com/intellij/openapi/editor/impl/DocumentImpl.java b/platform/core-impl/src/com/intellij/openapi/editor/impl/DocumentImpl.java index cc5f4e1e70aa..3624989ef0c3 100644 --- a/platform/core-impl/src/com/intellij/openapi/editor/impl/DocumentImpl.java +++ b/platform/core-impl/src/com/intellij/openapi/editor/impl/DocumentImpl.java @@ -34,10 +34,7 @@ import com.intellij.openapi.util.registry.Registry; import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.reference.SoftReference; -import com.intellij.util.DocumentUtil; -import com.intellij.util.IncorrectOperationException; -import com.intellij.util.LocalTimeCounter; -import com.intellij.util.Processor; +import com.intellij.util.*; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.IntArrayList; import com.intellij.util.text.CharArrayUtil; @@ -945,7 +942,7 @@ public class DocumentImpl extends UserDataHolderBase implements DocumentEx { private DocumentListener[] getCachedListeners() { DocumentListener[] cachedListeners = myCachedDocumentListeners.get(); if (cachedListeners == null) { - DocumentListener[] listeners = myDocumentListeners.toArray(new DocumentListener[myDocumentListeners.size()]); + DocumentListener[] listeners = ArrayUtil.stripTrailingNulls(myDocumentListeners.toArray(new DocumentListener[myDocumentListeners.size()])); Arrays.sort(listeners, PrioritizedDocumentListener.COMPARATOR); cachedListeners = listeners; myCachedDocumentListeners.set(cachedListeners); diff --git a/platform/core-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerContainerImpl.java b/platform/core-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerContainerImpl.java index cf243d08bce0..ed7529e039d9 100644 --- a/platform/core-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerContainerImpl.java +++ b/platform/core-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerContainerImpl.java @@ -189,12 +189,11 @@ public class VirtualFilePointerContainerImpl extends TraceableDisposable impleme result = EMPTY; } else { - VirtualFilePointer[] vf = myList.toArray(new VirtualFilePointer[myList.size()]); - List cachedFiles = new ArrayList(vf.length); - List cachedUrls = new ArrayList(vf.length); - List cachedDirectories = new ArrayList(vf.length / 3); + List cachedFiles = new ArrayList(myList.size()); + List cachedUrls = new ArrayList(myList.size()); + List cachedDirectories = new ArrayList(myList.size() / 3); boolean allFilesAreDirs = true; - for (VirtualFilePointer v : vf) { + for (VirtualFilePointer v : myList) { VirtualFile file = v.getFile(); String url = v.getUrl(); cachedUrls.add(url); diff --git a/platform/core-impl/src/com/intellij/psi/impl/PsiDocumentManagerBase.java b/platform/core-impl/src/com/intellij/psi/impl/PsiDocumentManagerBase.java index 4d7b44b8e7f7..b17d51de0c86 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/PsiDocumentManagerBase.java +++ b/platform/core-impl/src/com/intellij/psi/impl/PsiDocumentManagerBase.java @@ -43,10 +43,7 @@ import com.intellij.psi.impl.source.PsiFileImpl; import com.intellij.psi.impl.source.text.BlockSupportImpl; import com.intellij.psi.text.BlockSupport; import com.intellij.psi.util.PsiUtilCore; -import com.intellij.util.FileContentUtilCore; -import com.intellij.util.Processor; -import com.intellij.util.SmartList; -import com.intellij.util.SystemProperties; +import com.intellij.util.*; import com.intellij.util.concurrency.Semaphore; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.messages.MessageBus; @@ -574,7 +571,8 @@ public abstract class PsiDocumentManagerBase extends PsiDocumentManager implemen @NotNull public Document[] getUncommittedDocuments() { ApplicationManager.getApplication().assertIsDispatchThread(); - return myUncommittedDocuments.toArray(new Document[myUncommittedDocuments.size()]); + Document[] documents = myUncommittedDocuments.toArray(new Document[myUncommittedDocuments.size()]); + return ArrayUtil.stripTrailingNulls(documents); } boolean isInUncommittedSet(@NotNull Document document) { diff --git a/platform/lang-impl/src/com/intellij/codeInsight/intention/impl/config/IntentionManagerImpl.java b/platform/lang-impl/src/com/intellij/codeInsight/intention/impl/config/IntentionManagerImpl.java index ca0a51112f98..fcb5c4843781 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/intention/impl/config/IntentionManagerImpl.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/intention/impl/config/IntentionManagerImpl.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2009 JetBrains s.r.o. + * Copyright 2000-2014 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -37,6 +37,7 @@ import com.intellij.openapi.project.Project; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiFile; import com.intellij.util.Alarm; +import com.intellij.util.ArrayUtil; import com.intellij.util.IncorrectOperationException; import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NonNls; @@ -248,7 +249,7 @@ public class IntentionManagerImpl extends IntentionManager { @Override @NotNull public IntentionAction[] getIntentionActions() { - return myActions.toArray(new IntentionAction[myActions.size()]); + return ArrayUtil.stripTrailingNulls(myActions.toArray(new IntentionAction[myActions.size()])); } @NotNull diff --git a/platform/platform-impl/src/com/intellij/openapi/editor/impl/EditorFactoryImpl.java b/platform/platform-impl/src/com/intellij/openapi/editor/impl/EditorFactoryImpl.java index d0313e04cf9f..5bba8f308d40 100644 --- a/platform/platform-impl/src/com/intellij/openapi/editor/impl/EditorFactoryImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/editor/impl/EditorFactoryImpl.java @@ -37,6 +37,7 @@ import com.intellij.openapi.project.ProjectManager; import com.intellij.openapi.project.ProjectManagerAdapter; import com.intellij.openapi.util.Disposer; import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.util.ArrayUtil; import com.intellij.util.EventDispatcher; import com.intellij.util.SmartList; import com.intellij.util.containers.ContainerUtil; @@ -241,7 +242,7 @@ public class EditorFactoryImpl extends EditorFactory implements ApplicationCompo @Override @NotNull public Editor[] getAllEditors() { - return myEditors.toArray(new Editor[myEditors.size()]); + return ArrayUtil.stripTrailingNulls(myEditors.toArray(new Editor[myEditors.size()])); } @Override diff --git a/platform/util/src/com/intellij/util/ArrayUtil.java b/platform/util/src/com/intellij/util/ArrayUtil.java index 0b9e22822a9b..3d15e0265b03 100644 --- a/platform/util/src/com/intellij/util/ArrayUtil.java +++ b/platform/util/src/com/intellij/util/ArrayUtil.java @@ -24,6 +24,7 @@ import org.jetbrains.annotations.Nullable; import java.io.File; import java.lang.reflect.Array; +import java.util.Arrays; import java.util.Collection; import java.util.Comparator; import java.util.List; @@ -853,4 +854,18 @@ public class ArrayUtil extends ArrayUtilRt { dst[i++] = t; } } + + @NotNull + public static T[] stripTrailingNulls(T[] array) { + return array.length != 0 && array[array.length-1] == null ? Arrays.copyOf(array, trailingNullsIndex(array)) : array; + } + + private static int trailingNullsIndex(T[] array) { + for (int i=array.length-1; i>=0; i--) { + if (array[i] != null) { + return i+1; + } + } + return 0; + } } diff --git a/platform/util/src/com/intellij/util/containers/ContainerUtil.java b/platform/util/src/com/intellij/util/containers/ContainerUtil.java index 8a43e245aa14..2e37120c85a5 100644 --- a/platform/util/src/com/intellij/util/containers/ContainerUtil.java +++ b/platform/util/src/com/intellij/util/containers/ContainerUtil.java @@ -2242,6 +2242,9 @@ public class ContainerUtil extends ContainerUtilRt { * - faster modification in the uncontended case * - less memory * - slower modification in highly contented case (which is the kind of situation you shouldn't use COWAL anyway) + * + * N.B. Avoid using list.toArray(new T[list.size()]) on this list because it is inherently racey and + * therefore can return array with null elements at the end. */ @NotNull @Contract(pure=true) @@ -2330,6 +2333,9 @@ public class ContainerUtil extends ContainerUtilRt { return new ConcurrentWeakHashMap(initialCapacity, loadFactor, concurrencyLevel, hashingStrategy); } + /** + * @see {@link #createLockFreeCopyOnWriteList()} + */ @NotNull @Contract(pure=true) public static ConcurrentList createConcurrentList() { diff --git a/plugins/cvs/cvs-plugin/src/com/intellij/cvsSupport2/application/CvsEntriesManager.java b/plugins/cvs/cvs-plugin/src/com/intellij/cvsSupport2/application/CvsEntriesManager.java index 45e14c04399e..7c2f593b74e2 100644 --- a/plugins/cvs/cvs-plugin/src/com/intellij/cvsSupport2/application/CvsEntriesManager.java +++ b/plugins/cvs/cvs-plugin/src/com/intellij/cvsSupport2/application/CvsEntriesManager.java @@ -287,15 +287,13 @@ public class CvsEntriesManager extends VirtualFileAdapter { } private void onEntriesChanged(final VirtualFile parent) { - final CvsEntriesListener[] listeners = myEntriesListeners.toArray(new CvsEntriesListener[myEntriesListeners.size()]); - for (CvsEntriesListener listener : listeners) { + for (CvsEntriesListener listener : myEntriesListeners) { listener.entriesChanged(parent); } } private void onEntryChanged(final VirtualFile file) { - final CvsEntriesListener[] listeners = myEntriesListeners.toArray(new CvsEntriesListener[myEntriesListeners.size()]); - for (CvsEntriesListener listener : listeners) { + for (CvsEntriesListener listener : myEntriesListeners) { listener.entryChanged(file); } }