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.
Summary: | Extend FileOwnerQuery.markExternalOwner to handle individual files and non-existing files/roots | ||
---|---|---|---|
Product: | projects | Reporter: | Jan Lahoda <jlahoda> |
Component: | Generic Infrastructure | Assignee: | Jan Lahoda <jlahoda> |
Status: | RESOLVED FIXED | ||
Severity: | blocker | CC: | apireviews |
Priority: | P3 | Keywords: | API, API_REVIEW_FAST |
Version: | 5.x | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | ENHANCEMENT | Exception Reporter: | |
Bug Depends on: | 60297 | ||
Bug Blocks: | 57656 | ||
Attachments: |
Patch for registration of files and non-existing files and folders.
Improved patch for individual files and non-existent files/folders registration. Patch for issue #57656. |
Description
Jan Lahoda
2005-04-26 08:47:45 UTC
Registration of individual files is I think a no-brainer. Registration of nonexistent files/dirs is trickier. Currently the registry is a weak map from FileObject locations to FileObject owners. This works well because there is no real need to unregister owners explicitly - if the project is collected, and there are no other references to the external location, the map entry disappears automatically. If we permit nonexistent files/dirs, we need to keep a URI (or similar) as the key, and it is not obvious to me how the lifecycle of this should be managed. I guess if the value is also made a URI, we could just make it a non-weak map and just accept the very small memory leak this entails. Re registration of external roots: yes, this is not trivial. But, I think that this could be solved by a pair of WeakHashMaps: map1: URI->FileObject maps a (weakly referenced) URI to a weakly held FileObject (project directory) map2: FileObject->URI maps a (weakly referenced) FileObject (project directory) to the URI (not weak). Now, as long as the project directory's FileObject is strongly referenced, the URI is strongly referenced. Once the project directory's FO is collected, the URI is only weakly referenced and the mapping is removed. The assumption is that once noone holds the project directory, the project is removed/closed and the mapping is useless. The proposed trick would not work in case the initial registration is done using mEO(URI), unless that also tried to look up a FileObject for the URI. Don't know, might be enough, would have to analyze a bit more. Sorry, but I did not understand your last comment. I would propose to add method: markExternalOwner(URI root, Project owner, int algorithm) to the FileOwnerQuery. I do not see why the approach with two weak maps would not work in this case. I would do it this way: Map uri2Project = new WeakHashMap(); Map project2URI = new WeakHashMap(); uri2Project.put(root, new WeakReference(owner.getProjectDirectory())); project2URI.put(owner.getProjectDirectory(), new WeakReference(root)); Am I missing something? I will try to implement this, if you do not mind. In such a case the registration will simply be garbage collected in the next GC cycle, even if the owner project is held strongly elsewhere. FWIW, I don't currently believe it is possible to implement this correctly without *some* kind of memory leak. I can't prove it, but I think it is logically impossible. The best I can think of is to keep just one registry: private static final Map/*<URI,URI>*/ externalOwners = Collections.synchronizedMap(new HashMap()); Here the key is the the URI to the external root and the value is the URI to the project directory. The project itself can be collected and the entry will remain, but it will consume little space. Ooops, copy&paste mistake, sorry. The last line should read: project2URI.put(owner.getProjectDirectory(), root); In that case I think it works. If the project is closed and collected, the mapping will be removed, unless and until it is reopened, which is acceptable. Attaching patch that allows registration of individual files and non-existing folders/files. Created attachment 22127 [details]
Patch for registration of files and non-existing files and folders.
Patch to projectapi/nbproject/project.properties is bogus. Don't call Thread.sleep(30000) in the test; set static { TimedWeakReference.TIMEOUT = 0; } fileObject2URI in the test may more simply just throw Exception. In the main class, a FileStateInvalidException probably should be rethrown as an IllegalArgumentException (since no one should pass an invalid FileObject in); and you can use URI.create rather than new URI(...) (no checked exc.) since URL.toString() ought to yield a valid URI in all cases. Otherwise looks good to me. Are you going to submit to apireviews? Yes, I will correct the patch and then I will send it to apireviews. Created attachment 22142 [details]
Improved patch for individual files and non-existent files/folders registration.
Created attachment 22143 [details]
Patch for issue #57656.
I would like to ask for API review for this change. The proposed patch is attached as 58313c.diff. The change is used by fix of issue #57656, attached is also diff that solves this issue (57656.diff) for reference. If there are no objections, I will commit the change tomorrow. For the issue #57656 part, I disagree with the approach taken. (Please attach the patch to that issue and evaluate it separately.) Prefer to keep the existing style for defining external files as is already used for folders, so that the project.xml is explicit about what it does, and <export> is used to control only AntArtifactQuery; so need to add <build-file> to the general schema, analogous to <build-folder>. This means creating a new rev of the general freeform schema and making sure both old and new schemas are interpreted. The GUI customizer should already be defining <build-folder> in project.xml when you set an output location for some source root which is not inside the project dir. This should be extended to define either <build-folder> and/or <build-file> (depending on whether e.g. FileUtil.isArchiveFile on each location); the customizer should write a new-schema project.xml iff <build-file> would be needed (must warn user of this as for the j2seproject upgrade); and the SourcesHelper used in the freeform project should be configured from both <build-file> and <build-folder>. Also need unit test patch for new SourcesHelper behavior. Patch for this issue looks OK to me. BTW spelling: "collectible" not "collectable" (please don't ask me why). Implemented: Checking in projectapi/apichanges.xml; /cvs/projects/projectapi/apichanges.xml,v <-- apichanges.xml new revision: 1.3; previous revision: 1.2 done Checking in projectapi/manifest.mf; /cvs/projects/projectapi/manifest.mf,v <-- manifest.mf new revision: 1.8; previous revision: 1.7 done Checking in projectapi/src/org/netbeans/api/project/FileOwnerQuery.java; /cvs/projects/projectapi/src/org/netbeans/api/project/FileOwnerQuery.java,v <-- FileOwnerQuery.java new revision: 1.7; previous revision: 1.6 done Checking in projectapi/src/org/netbeans/modules/projectapi/SimpleFileOwnerQueryImplementation.java; /cvs/projects/projectapi/src/org/netbeans/modules/projectapi/SimpleFileOwnerQueryImplementation.java,v <-- SimpleFileOwnerQueryImplementation.java new revision: 1.9; previous revision: 1.8 done Checking in projectapi/test/unit/src/org/netbeans/api/project/FileOwnerQueryTest.java; /cvs/projects/projectapi/test/unit/src/org/netbeans/api/project/FileOwnerQueryTest.java,v <-- FileOwnerQueryTest.java new revision: 1.8; previous revision: 1.7 done |