This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Created attachment 114792 [details] thread dump Product Version: NetBeans IDE 7.0.1 (Build 201107282000) Java: 1.6.0_26; Java HotSpot(TM) Client VM 20.1-b02 After closing project in Projects tab the IDE does not respond any more.
"Opening projects" daemon prio=10 tid=0x09e9ec00 nid=0x131a waiting for monitor entry [0xa9cfb000] at org.netbeans.modules.maven.NbMavenProjectImpl$LazyLookup.beforeLookup(NbMavenProjectImpl.java:844) - waiting to lock <0x5d03ff18> (a org.netbeans.modules.maven.NbMavenProjectImpl) at org.openide.util.lookup.ProxyLookup.lookup(ProxyLookup.java:206) at org.netbeans.api.project.ProjectUtils.getInformation(ProjectUtils.java:101) ... at org.netbeans.modules.project.ui.OpenProjectList$8.run(OpenProjectList.java:776) at org.openide.util.Mutex.writeAccess(Mutex.java:397) "Parsing & Indexing Loop (201107282000)" daemon prio=10 tid=0xaaddb800 nid=0xa1e in Object.wait() [0xb31fd000] ... at org.openide.util.Mutex.readAccess(Mutex.java:327) at org.netbeans.api.project.ProjectManager.findProject(ProjectManager.java:233) ... at org.netbeans.modules.maven.NbMavenProjectImpl.loadOriginalMavenProject(NbMavenProjectImpl.java:372) at org.netbeans.modules.maven.NbMavenProjectImpl.getOriginalMavenProject(NbMavenProjectImpl.java:344) - locked <0x5d03ff18> (a org.netbeans.modules.maven.NbMavenProjectImpl) at org.netbeans.modules.maven.NbMavenProjectImpl.getArtifactRelativeRepositoryPath(NbMavenProjectImpl.java:633)
The only solution is to prevent the NbArtifactFixer to call FOQ->ProjectManager. I suppose we could attempt to externalize the loading of MavenProject instances and make it possible without holding an instance of NbMavenProjectImpl..
(In reply to comment #2) > we could attempt to externalize the loading of MavenProject instances > and make it possible without holding an instance of NbMavenProjectImpl You mean have MFOQI.getOwnerPOM load a plain MavenProject rather than a (NB) Project, by changing getOwner(String,String,String) to return the projectDir instead of p. It would be nice, but then we would need to have some way of caching loaded MavenProject's other than as a field like NbMPI.project (and knowing when to expire the cache). Might be simpler to address the actual lock ordering conflict shown here. Not sure what that was, since LazyLookup no longer exists - this particular deadlock may be impossible in current code.
Created attachment 120050 [details] proposed patch
Created attachment 120052 [details] corrected patch
(In reply to comment #3) > (In reply to comment #2) > > we could attempt to externalize the loading of MavenProject instances > > and make it possible without holding an instance of NbMavenProjectImpl > > You mean have MFOQI.getOwnerPOM load a plain MavenProject rather than a (NB) > Project, by changing getOwner(String,String,String) to return the projectDir > instead of p. It would be nice, but then we would need to have some way of > caching loaded MavenProject's other than as a field like NbMPI.project (and > knowing when to expire the cache). to expire is fairly easy IMHO.. projects use SoftReference, when using WeakReference in the cache we should release the not belonging to project ones early and and the rest will only go when project goes. > > Might be simpler to address the actual lock ordering conflict shown here. Not > sure what that was, since LazyLookup no longer exists - this particular > deadlock may be impossible in current code. Well, the general problem persists. we lock on accessing the MavenProject instance and have no influence on whether someone called us without a project mutex, with a read or write one.. and then inside the project loading we access ProjectManager.findProject().. the only reason why we don't get the deadlock more often is that expressions in GAVs are fairly rare.
(In reply to comment #5) > Created attachment 120052 [details] > corrected patch this is the correct patch meant for review. 1. I've removed all dependencies on projects from loading the pom. Now we load configurations in a different way when loading the pom and differently later on inside the project. A nice sideeffect was the removal of the MavenEmbedder.createModelLineage before the actual project loading.. 2. the cache itself references MavenProject instances using WeakReference. Open question is how to reference project dir FileObjects (used as keys in the maps). Currently they are hard referenced. 3. The MavenExecutionResult instance used by the MavenProject's ProblemReporter is attached to the MavenProject instance itself, if loaded first by FOQ and later by Project, we still can populate the problemreporter 4. possible further speed improvement in FOQ is to check if the cache contains a resolved value before executing the ModelReader please review
(In reply to comment #7) > this is the correct patch meant for review. BTW use the "obsoletes" option in BZ to hide older patches. For clean patches, please delete obsolete sections of code (e.g. "} catch (RuntimeException exc) {...") rather than commenting them out. Implementing an interface merely in order to import its constants is considered bad style; use a class with a private constructor and public static final fields, and import static them if necessary. > how to reference project dir FileObjects (used as keys in the > maps). Currently they are hard referenced. This is going to be a memory leak. I think a WeakHashMap<FileObject,SoftReference<MavenProject>> would suffice for file2Project. (The MavenProject should not be strongly held since NbMPI.project holds it only softly: bug #192155.) file2Mutex and getMutex probably do not need to exist - just synchronize on file2Project. getMavenProject should be marked @NonNull for clarity - it is not obvious that this returns a value even for a dir which does not contain any pom.xml at all. The rest of the patch (configurations, etc.) I do not follow well enough to review.
(In reply to comment #8) > > Implementing an interface merely in order to import its constants is considered > bad style; use a class with a private constructor and public static final > fields, and import static them if necessary. done. > > > how to reference project dir FileObjects (used as keys in the > > maps). Currently they are hard referenced. > > This is going to be a memory leak. I think a > WeakHashMap<FileObject,SoftReference<MavenProject>> would suffice for > file2Project. (The MavenProject should not be strongly held since NbMPI.project > holds it only softly: bug #192155.) Well, as long as the Project instance is around (which hard references the project's FileObject) WeakReference is as good as SoftReference for referencing the MavenProject. WeakMapping the FileObject might free the instances referenced via NBArtifactFixer early on, we only use the FO locally in a method, so unless the FileObject is held elsewhere the Map entry will be GCed via the WeakMap path. Here the unknown for me is how are FileObject instances referenced and de-referenced. > > file2Mutex and getMutex probably do not need to exist - just synchronize on > file2Project. That's going to create a bottleneck. Previously the synchronization was done on per-Project basis, now it would be global. > > getMavenProject should be marked @NonNull for clarity - it is not obvious that > this returns a value even for a dir which does not contain any pom.xml at all. > I've fixed that by returning null early on to prevent having the cache flooded with fallback poms for non-existing projects.
http://hg.netbeans.org/core-main/rev/d6b1ad61ed81
(In reply to comment #9) > WeakReference is as good as SoftReference for referencing > the MavenProject. Agreed. > WeakMapping the FileObject might free the instances > referenced via NBArtifactFixer early on Yes, it could, but this is preferable to holding irrelevant files forever. (o.n.m.projectapi.TimedWeakReference is a handy compromise; maybe it should be made a utility API?) >> just synchronize on file2Project. > > That's going to create a bottleneck. Of course, but that is not necessarily bad - unless you really expect multiple hardware threads to be loading MavenProject's instances in parallel. Since this job is likely I/O-bound I would expect that to be slower than simple serial loading. > returning null early on to prevent having the cache flooded > with fallback poms for non-existing projects. Right, that is probably better.
(In reply to comment #11) > (In reply to comment #9) > >> just synchronize on file2Project. > > > > That's going to create a bottleneck. > > Of course, but that is not necessarily bad - unless you really expect multiple > hardware threads to be loading MavenProject's instances in parallel. Since this > job is likely I/O-bound I would expect that to be slower than simple serial > loading. > it's the scary reality that requests to load MavenProject instances come from all directions, including AWT. (FileEncodingQuery etc.). it's possible that in sum of all loading times the serial loading is faster, but will be likely slower for these critical case (IMHO). We've had non-serial loading so far, that's another reason I'm more comfortable with that approach (the known evil beats unknown one)
Integrated into 'main-golden', will be available in build *201206050001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-golden/rev/d6b1ad61ed81 User: Milos Kleint <mkleint@netbeans.org> Log: #207172 have MavenProject instances loaded without need for a Project, keep external cache of such instances and call it from both NbMavenProjectImpl and NbArtifactFixer, that way we get rid of entering ProjectMutex while loading the MavenProject instances (via NbArtifactFixer and MavenFileOwnerQueryImpl)