# HG changeset patch # Parent f4f39e2374dcbfc168b368a0eb3f7389ee08ca7c #187657: deadlock between OpenProjectList$RecentProjectList & OpenProjectList.class. Simplify threading in this class by using ProjectManager.mutex consistently throughout. diff --git a/projectui/src/org/netbeans/modules/project/ui/OpenProjectList.java b/projectui/src/org/netbeans/modules/project/ui/OpenProjectList.java --- a/projectui/src/org/netbeans/modules/project/ui/OpenProjectList.java +++ b/projectui/src/org/netbeans/modules/project/ui/OpenProjectList.java @@ -77,6 +77,8 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -223,16 +225,18 @@ // Implementation of the class --------------------------------------------- public static OpenProjectList getDefault() { - synchronized ( OpenProjectList.class ) { - if ( INSTANCE == null ) { - INSTANCE = new OpenProjectList(); - INSTANCE.openProjects = loadProjectList(); - // Load recent project list - INSTANCE.recentProjects.load(); - WindowManager.getDefault().invokeWhenUIReady(INSTANCE.LOAD); + return ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override OpenProjectList run() { + if (INSTANCE == null) { + INSTANCE = new OpenProjectList(); + INSTANCE.openProjects = loadProjectList(); + // Load recent project list + INSTANCE.recentProjects.load(); + WindowManager.getDefault().invokeWhenUIReady(INSTANCE.LOAD); + } + return INSTANCE; } - } - return INSTANCE; + }); } static void waitProjectsFullyOpen() { @@ -264,11 +268,11 @@ } /** Modifications to the recentTemplates variables shall be done only - * when hodling a lock. + * when holding a lock. * @return the list */ private List getRecentTemplates() { - assert Thread.holdsLock(this); + assert ProjectManager.mutex().isReadAccess() || ProjectManager.mutex().isWriteAccess(); return recentTemplates; } @@ -341,8 +345,9 @@ } } - final void preferredProject(Project lazyP) { - synchronized (toOpenProjects) { + final void preferredProject(final Project lazyP) { + ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Void run() { for (Project p : toOpenProjects) { FileObject dir = p.getProjectDirectory(); assert dir != null : "Project has real directory " + p; @@ -352,15 +357,18 @@ if (dir.equals(lazyP.getProjectDirectory())) { toOpenProjects.remove(p); toOpenProjects.addFirst(p); - return; + return null; } } - } + return null; + } + }); } private void updateGlobalState() { log(Level.FINER, "updateGlobalState"); // NOI18N - synchronized (INSTANCE) { + ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Void run() { INSTANCE.openProjects = lazilyOpenedProjects; log(Level.FINER, "openProjects changed: {0}", lazilyOpenedProjects); // NOI18N if (lazyMainProject != null) { @@ -369,7 +377,9 @@ INSTANCE.mainProject = unwrapProject(INSTANCE.mainProject); INSTANCE.getRecentTemplates().addAll(recentTemplates); log(Level.FINER, "updateGlobalState, applied"); // NOI18N + return null; } + }); INSTANCE.pchSupport.firePropertyChange(PROPERTY_OPEN_PROJECTS, new Project[0], lazilyOpenedProjects.toArray(new Project[0])); INSTANCE.pchSupport.firePropertyChange(PROPERTY_MAIN_PROJECT, null, INSTANCE.mainProject); @@ -377,36 +387,41 @@ log(Level.FINER, "updateGlobalState, done, notified"); // NOI18N } - boolean closeBeforeOpen(Project[] arr) { - NEXT: for (int i = 0; i < arr.length; i++) { - FileObject dir = arr[i].getProjectDirectory(); - synchronized (toOpenProjects) { - for (Iterator it = toOpenProjects.iterator(); it.hasNext();) { - if (dir.equals(it.next().getProjectDirectory())) { - it.remove(); - continue NEXT; + boolean closeBeforeOpen(final Project[] arr) { + return ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Boolean run() { + NEXT: for (Project p : arr) { + FileObject dir = p.getProjectDirectory(); + for (Iterator it = toOpenProjects.iterator(); it.hasNext();) { + if (dir.equals(it.next().getProjectDirectory())) { + it.remove(); + continue NEXT; + } } + return false; } - return false; + return true; } - } - return true; + }); } private void loadOnBackground() { lazilyOpenedProjects = new ArrayList(); List URLs = OpenProjectListSettings.getInstance().getOpenProjectsURLs(); - Project[] inital; + final List initial = new ArrayList(); final LinkedList projects = URLs2Projects(URLs); - synchronized (toOpenProjects) { - toOpenProjects.addAll(projects); - log(Level.FINER, "loadOnBackground {0}", toOpenProjects); // NOI18N - inital = toOpenProjects.toArray(new Project[0]); + ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Void run() { + toOpenProjects.addAll(projects); + log(Level.FINER, "loadOnBackground {0}", toOpenProjects); // NOI18N + initial.addAll(toOpenProjects); + return null; } + }); recentTemplates = new ArrayList( OpenProjectListSettings.getInstance().getRecentTemplates() ); - URL mainProjectURL = OpenProjectListSettings.getInstance().getMainProjectURL(); - int max; - synchronized (toOpenProjects) { + final URL mainProjectURL = OpenProjectListSettings.getInstance().getMainProjectURL(); + int max = ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Integer run() { for (Project p : toOpenProjects) { INSTANCE.addModuleInfo(p); // Set main project @@ -420,19 +435,25 @@ // Not a main project } } - max = toOpenProjects.size(); - } + return toOpenProjects.size(); + } + }); progress.switchToDeterminate(max); for (;;) { - Project p; - int openPrjSize; - synchronized (toOpenProjects) { - if (toOpenProjects.isEmpty()) { - break; + final AtomicInteger openPrjSize = new AtomicInteger(); + Project p = ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Project run() { + if (toOpenProjects.isEmpty()) { + return null; + } + Project p = toOpenProjects.remove(); + log(Level.FINER, "after remove {0}", toOpenProjects); // NOI18N + openPrjSize.set(toOpenProjects.size()); + return p; } - p = toOpenProjects.remove(); - log(Level.FINER, "after remove {0}", toOpenProjects); // NOI18N - openPrjSize = toOpenProjects.size(); + }); + if (p == null) { + break; } log(Level.FINE, "about to open a project {0}", p); // NOI18N if (notifyOpened(p)) { @@ -451,12 +472,13 @@ lazyMainProject = null; } } - progress.progress(max - openPrjSize); + progress.progress(max - openPrjSize.get()); } - if (inital != null) { - log(createRecord("UI_INIT_PROJECTS", inital),"org.netbeans.ui.projects"); - log(createRecordMetrics("USG_PROJECT_OPEN", inital),"org.netbeans.ui.metrics.projects"); + if (initial != null) { + Project[] initialA = initial.toArray(new Project[initial.size()]); + log(createRecord("UI_INIT_PROJECTS", initialA),"org.netbeans.ui.projects"); + log(createRecordMetrics("USG_PROJECT_OPEN", initialA),"org.netbeans.ui.metrics.projects"); } } @@ -687,9 +709,12 @@ final List oldprjs = new ArrayList(); final List newprjs = new ArrayList(); - synchronized (this) { - oldprjs.addAll(openProjects); - } + ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Void run() { + oldprjs.addAll(openProjects); + return null; + } + }); for (Project p: projectsToOpen) { @@ -707,14 +732,18 @@ handle.progress((int) currentWork); } } - - synchronized ( this ) { - newprjs.addAll(openProjects); - saveProjectList( openProjects ); - if ( recentProjectsChanged ) { - recentProjects.save(); + + final boolean _recentProjectsChanged = recentProjectsChanged; + ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Void run() { + newprjs.addAll(openProjects); + saveProjectList(openProjects); + if (_recentProjectsChanged) { + recentProjects.save(); + } + return null; } - } + }); if (handle != null) { handle.finish(); @@ -750,7 +779,7 @@ LOAD.waitFinished(); } - Project[] projects = new Project[someProjects.length]; + final Project[] projects = new Project[someProjects.length]; for (int i = 0; i < someProjects.length; i++) { projects[i] = unwrapProject(someProjects[i]); } @@ -764,18 +793,19 @@ LOAD.enter(); ProjectUtilities.WaitCursor.show(); logProjects("close(): closing project: ", projects); - boolean mainClosed = false; - boolean someClosed = false; - List oldprjs = new ArrayList(); - List newprjs = new ArrayList(); + final AtomicBoolean mainClosed = new AtomicBoolean(); + final AtomicBoolean someClosed = new AtomicBoolean(); + final List oldprjs = new ArrayList(); + final List newprjs = new ArrayList(); final List notifyList = new ArrayList(); - synchronized ( this ) { + ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Void run() { oldprjs.addAll(openProjects); - for( int i = 0; i < projects.length; i++ ) { + for (Project p : projects) { Iterator it = openProjects.iterator(); boolean found = false; while (it.hasNext()) { - if (it.next().equals(projects[i])) { + if (it.next().equals(p)) { found = true; break; } @@ -783,28 +813,30 @@ if (!found) { continue; // Nothing to remove } - if ( !mainClosed ) { - mainClosed = isMainProject( projects[i] ); + if (!mainClosed.get()) { + mainClosed.set(isMainProject(p)); } // remove the project from openProjects it.remove(); - removeModuleInfo(projects[i]); + removeModuleInfo(p); - projects[i].getProjectDirectory().removeFileChangeListener(deleteListener); + p.getProjectDirectory().removeFileChangeListener(deleteListener); - notifyList.add(projects[i]); + notifyList.add(p); - someClosed = true; + someClosed.set(true); } - if ( someClosed ) { + if (someClosed.get()) { newprjs.addAll(openProjects); saveProjectList(openProjects); } - if ( mainClosed ) { - this.mainProject = null; + if (mainClosed.get()) { + mainProject = null; saveMainProject( mainProject ); } + return null; } + }); if (!notifyList.isEmpty()) { for (Project p : notifyList) { recentProjects.add(p); // #183681: call outside of lock @@ -820,14 +852,14 @@ } }); logProjects("close(): openProjects == ", openProjects.toArray(new Project[0])); // NOI18N - if ( someClosed ) { + if (someClosed.get()) { pchSupport.firePropertyChange( PROPERTY_OPEN_PROJECTS, oldprjs.toArray(new Project[oldprjs.size()]), newprjs.toArray(new Project[newprjs.size()]) ); } - if ( mainClosed ) { + if (mainClosed.get()) { pchSupport.firePropertyChange( PROPERTY_MAIN_PROJECT, null, null ); } - if ( someClosed ) { + if (someClosed.get()) { pchSupport.firePropertyChange( PROPERTY_RECENT_PROJECTS, null, null ); } if (doSave) { @@ -854,59 +886,71 @@ } } - public synchronized Project[] getOpenProjects() { - Project projects[] = new Project[ openProjects.size() ]; - openProjects.toArray( projects ); - return projects; + public Project[] getOpenProjects() { + return ProjectManager.mutex().readAccess(new Mutex.Action() { + public @Override Project[] run() { + return openProjects.toArray(new Project[openProjects.size()]); + } + }); } - public synchronized boolean isOpen( Project p ) { + public boolean isOpen(final Project p) { // XXX shouldn't this just use openProjects.contains(p)? - for(Project cp : openProjects) { - if ( p.getProjectDirectory().equals( cp.getProjectDirectory() ) ) { - return true; + return ProjectManager.mutex().readAccess(new Mutex.Action() { + public @Override Boolean run() { + for (Project cp : openProjects) { + if (p.getProjectDirectory().equals(cp.getProjectDirectory())) { + return true; + } + } + return false; } - } - return false; + }); } - public synchronized boolean isMainProject( Project p ) { - - if ( mainProject != null && p != null && - mainProject.getProjectDirectory().equals( p.getProjectDirectory() ) ) { - return true; - } - else { - return false; - } - + public boolean isMainProject(final Project p) { + return ProjectManager.mutex().readAccess(new Mutex.Action() { + public @Override Boolean run() { + if (mainProject != null && p != null && mainProject.getProjectDirectory().equals(p.getProjectDirectory())) { + return true; + } else { + return false; + } + } + }); } - public synchronized Project getMainProject() { - return mainProject; + public Project getMainProject() { + return ProjectManager.mutex().readAccess(new Mutex.Action() { + public @Override Project run() { + return mainProject; + } + }); } - public void setMainProject( Project mainProject ) { + public void setMainProject( final Project mainProject ) { LOGGER.finer("Setting main project: " + mainProject); // NOI18N logProjects("setMainProject(): openProjects == ", openProjects.toArray(new Project[0])); // NOI18N - synchronized ( this ) { - if (mainProject != null && !openProjects.contains(mainProject)) { + ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Void run() { + Project main = mainProject; + if (main != null && !openProjects.contains(main)) { //#139965 the project passed in here can be different from the current one. // eg when the ManProjectAction shows a list of opened projects, it lists the "non-loaded skeletons" // but when the user eventually selects one, the openProjects list already might hold the // correct loaded list. try { - mainProject = ProjectManager.getDefault().findProject(mainProject.getProjectDirectory()); - if (mainProject != null) { + main = ProjectManager.getDefault().findProject(main.getProjectDirectory()); + if (main != null) { boolean fail = true; for (Project p : openProjects) { - if (p.equals(mainProject)) { + if (p.equals(main)) { fail = false; break; } if (p instanceof LazyProject) { - if (p.getProjectDirectory().equals(mainProject.getProjectDirectory())) { - mainProject = p; + if (p.getProjectDirectory().equals(main.getProjectDirectory())) { + main = p; fail = false; break; } @@ -922,18 +966,18 @@ } } - this.mainProject = mainProject; - saveMainProject( mainProject ); + OpenProjectList.this.mainProject = main; + saveMainProject(main); + return null; } + }); pchSupport.firePropertyChange( PROPERTY_MAIN_PROJECT, null, null ); } public List getRecentProjects() { return ProjectManager.mutex().readAccess(new Mutex.Action>() { public List run() { - synchronized (OpenProjectList.class) { - return recentProjects.getProjects(); - } + return recentProjects.getProjects(); } }); } @@ -941,9 +985,7 @@ public boolean isRecentProjectsEmpty() { return ProjectManager.mutex().readAccess(new Mutex.Action() { public Boolean run() { - synchronized (OpenProjectList.class) { - return recentProjects.isEmpty(); - } + return recentProjects.isEmpty(); } }); } @@ -951,9 +993,7 @@ public List getRecentProjectsInformation() { return ProjectManager.mutex().readAccess(new Mutex.Action>() { public List run() { - synchronized (OpenProjectList.class) { - return recentProjects.getRecentProjectsInfo(); - } + return recentProjects.getRecentProjectsInfo(); } }); } @@ -997,7 +1037,9 @@ // Used from NewFile action - public synchronized void updateTemplatesLRU( FileObject template ) { + public void updateTemplatesLRU(final FileObject template) { + ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Void run() { String templateName = template.getPath(); @@ -1011,6 +1053,9 @@ } OpenProjectListSettings.getInstance().setRecentTemplates( new ArrayList( getRecentTemplates() ) ); + return null; + } + }); } @@ -1159,9 +1204,9 @@ } private boolean doOpenProject(final Project p) { - boolean recentProjectsChanged; LOGGER.finer("doOpenProject(): opening project " + p.toString()); - synchronized (this) { + boolean recentProjectsChanged = ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Boolean run() { log(Level.FINER, "already opened: {0} ", openProjects); for (Project existing : openProjects) { if (p.equals(existing) || existing.equals(p)) { @@ -1174,8 +1219,9 @@ p.getProjectDirectory().addFileChangeListener(deleteListener); p.getProjectDirectory().addFileChangeListener(nbprojectDeleteListener); - recentProjectsChanged = recentProjects.remove(p); + return recentProjects.remove(p); } + }); logProjects("doOpenProject(): openProjects == ", openProjects.toArray(new Project[0])); // NOI18N // Notify projects opened notifyOpened(p); @@ -1238,24 +1284,25 @@ } } - private ArrayList getTemplateNamesLRU( Project project, PrivilegedTemplates priv ) { + private ArrayList getTemplateNamesLRU( final Project project, PrivilegedTemplates priv ) { // First take recently used templates and try to find those which // are supported by the project. - ArrayList result = new ArrayList(NUM_TEMPLATES); + final ArrayList result = new ArrayList(NUM_TEMPLATES); RecommendedTemplates rt = project.getLookup().lookup( RecommendedTemplates.class ); String rtNames[] = rt == null ? new String[0] : rt.getRecommendedTypes(); PrivilegedTemplates pt = priv != null ? priv : project.getLookup().lookup( PrivilegedTemplates.class ); String ptNames[] = pt == null ? null : pt.getPrivilegedTemplates(); - ArrayList privilegedTemplates = new ArrayList( Arrays.asList( pt == null ? new String[0]: ptNames ) ); + final ArrayList privilegedTemplates = new ArrayList( Arrays.asList( pt == null ? new String[0]: ptNames ) ); if (priv == null) { // when the privileged templates are part of the active lookup, // do not mix them with the recent templates, but use only the privileged ones. // eg. on Webservices node, one is not interested in a recent "jsp" file template.. - synchronized (this) { + ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Void run() { Iterator it = getRecentTemplates().iterator(); for( int i = 0; i < NUM_TEMPLATES && it.hasNext(); i++ ) { String templateName = it.next(); @@ -1271,7 +1318,9 @@ continue; } } + return null; } + }); } // If necessary fill the list with the rest of privileged templates @@ -1352,8 +1401,8 @@ } } - public void add(Project p) { - UnloadedProjectInformation projectInfo; + public void add(final Project p) { + final UnloadedProjectInformation projectInfo; try { // #183681: call outside of lock projectInfo = ProjectInfoAccessor.DEFAULT.getProjectInfo( ProjectUtils.getInformation(p).getDisplayName(), @@ -1363,7 +1412,8 @@ ErrorManager.getDefault().notify(ErrorManager.INFORMATIONAL, ex); return; } - synchronized (this) { + ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Void run() { int index = getIndex(p); if (index == -1) { // Project not in list @@ -1383,10 +1433,14 @@ } recentProjects.add(0, new ProjectReference(p)); recentProjectsInfos.add(0, projectInfo); + return null; } + }); } - public synchronized boolean remove( Project p ) { + public boolean remove(final Project p) { + return ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Boolean run() { int index = getIndex( p ); if ( index != -1 ) { LOGGER.log(Level.FINE, "remove recent project: {0} @{1}", new Object[] {p, index}); @@ -1395,12 +1449,13 @@ return true; } return false; + } + }); } public void refresh() { - ProjectManager.mutex().readAccess(new Runnable() { + ProjectManager.mutex().writeAccess(new Runnable() { public void run () { - synchronized (RecentProjectList.this) { assert recentProjects.size() == recentProjectsInfos.size(); boolean refresh = false; Iterator recentProjectsIter = recentProjects.iterator(); @@ -1438,7 +1493,6 @@ pchSupport.firePropertyChange(PROPERTY_RECENT_PROJECTS, null, null); save(); } - } } }); } @@ -1473,7 +1527,9 @@ return empty; } - public synchronized void load() { + public void load() { + ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Void run() { List URLs = OpenProjectListSettings.getInstance().getRecentProjectsURLs(); List names = OpenProjectListSettings.getInstance().getRecentProjectsDisplayNames(); List icons = OpenProjectListSettings.getInstance().getRecentProjectsIcons(); @@ -1507,6 +1563,9 @@ assert p.getProjectDirectory() != null : "Project " + p + " has null project directory"; p.getProjectDirectory().addFileChangeListener(nbprojectDeleteListener); } + return null; + } + }); } public void save() { @@ -1561,10 +1620,14 @@ return -1; } - private synchronized List getRecentProjectsInfo() { + private List getRecentProjectsInfo() { // #166408: refreshing is too time expensive and we want to be fast, not correct //refresh(); - return new ArrayList(recentProjectsInfos); + return ProjectManager.mutex().readAccess(new Mutex.Action>() { + public @Override List run() { + return new ArrayList(recentProjectsInfos); + } + }); } private class ProjectReference { @@ -1701,14 +1764,15 @@ } /** - * Closesdeleted projects. + * Closes deleted projects. */ private final class ProjectDeletionListener extends FileChangeAdapter { public ProjectDeletionListener() {} - public @Override void fileDeleted(FileEvent fe) { - synchronized (OpenProjectList.this) { + public @Override void fileDeleted(final FileEvent fe) { + ProjectManager.mutex().readAccess(new Mutex.Action() { + public @Override Void run() { Project toRemove = null; for (Project prj : openProjects) { if (fe.getFile().equals(prj.getProjectDirectory())) { @@ -1726,7 +1790,9 @@ } }); } - } + return null; + } + }); } } @@ -1747,17 +1813,20 @@ return info; } - private void addModuleInfo(Project prj) { - ModuleInfo info = findModuleForProject(prj); + private void addModuleInfo(final Project prj) { + final ModuleInfo info = findModuleForProject(prj); if (info != null) { // is null in tests.. - synchronized (openProjectsModuleInfos) { + ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Void run() { if (!openProjectsModuleInfos.containsKey(info)) { openProjectsModuleInfos.put(info, new ArrayList()); info.addPropertyChangeListener(infoListener); } openProjectsModuleInfos.get(info).add(prj); + return null; } + }); } } @@ -1766,19 +1835,22 @@ removeModuleInfo(prj, info); } - private void removeModuleInfo(Project prj, ModuleInfo info) { + private void removeModuleInfo(final Project prj, final ModuleInfo info) { // info can be null in case we are closing a project from disabled module if (info != null) { - synchronized (openProjectsModuleInfos) { + ProjectManager.mutex().writeAccess(new Mutex.Action() { + public @Override Void run() { List prjlist = openProjectsModuleInfos.get(info); if (prjlist != null) { prjlist.remove(prj); - if (prjlist.size() == 0) { + if (prjlist.isEmpty()) { info.removePropertyChangeListener(infoListener); openProjectsModuleInfos.remove(info); } } + return null; } + }); } }