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.
The behavior of ProjectManager.mutex has incompatible changed causing j2seproject tests to fail. For now I put workaround to let at least the tests pass. The problem is that file changes caused in the project mutex critical section are not visible to the code in the same critical section (missing events).
Created attachment 69081 [details] Rough fix of Tomáš's test
Created attachment 69236 [details] If the original test used EditableProperties, it would pass now
I guess now is the time for you to add few comments about ugliness... I do not want to compromise your code, but if I am supposed to propose a low risk fix at this stage of development, which does not change anything, just makes the Tomáš's test pass, the change would look like http://www.netbeans.org/nonav/issues/showattachment.cgi/69236/EditablePropertiesDeliverEventsImmediatelly.diff It delivers events synchronously (without use of RequestProcessor), plus if one uses EditableProperties.store(fo), it delivers them immediately. This makes Tomáš's test pass without adding any ugliness on the side of his API usage. The implementation requires review from people who know how things are supposed to work. It adds one desirable API change and one unwanted, yet needed one. There are some tests, more would be beneficial. That is the whole prototype.
Created attachment 69326 [details] Diff ready for pre-integration review
The last diff http://www.netbeans.org/nonav/issues/showattachment.cgi/69326/EditablePropertiesDeliverEventsImmediatelly.diff is documented enough to be ready for API review, the tests of all three affected modules are OK (except issue 146525, but that is unrelated to this change) and the change seeks minimal influence to 6.1 behaviour. I guess it is up to Milan to get comfortable with the change and send it to API review.
Milan feels it is OK to get comments from everyone.
Yes, please.
The patch looks fine. I only want to point out that it may cause some problems in code depending on non coming events in writeAccess, but this is expected and I doubt there are any. I would prefer the method isMutexEvent to be in SPI/support since it is not probably first order API and looks strange.
I still consider all this a workaround for a bug in ProjectManager, but it will have to do for now. Would rather not have isMutexEvent document that PM is obligated to delay delivery of FS events. BTW typo: 'genereated' [JG01] What is the purpose of the patch to J2SEConfigurationProvider.java? It just looks to rename and move a method for no reason. [JG02] Please do not add EditableProperties.store(FileObject) to the API. EP is intended to be a pure utility class similar to java.util.Properties, destined ultimately for some generic utilities module along with EditableManifest, and a dependency on FileObject is unwelcome. The dependency on FileChangeSupport is certainly not acceptable. You can add this method to org.netbeans.modules.project.ant.Util (this will be visible to unit tests so no general API addition is needed). Anyway the body is too long and complicated; you maybe copied it from some old clumsy code. It need only do OutputStream os = fo.getOutputStream(); try { ep.store(os); } finally { os.close(); } [JG03] Why is apichanges talking about AntProjectHelper.createSimpleAntArtifact? [JG04] Using System.currentTimeMillis() to implement a sequence number is a bad idea; you do not know what the clock tick is. Use an AtomicInteger if you need a logical timestamp. [JG05] Please revert the apparently bogus indentation changes in GeneratedFilesHelper.java. [JG06] Delete the suite() method from FileChangeSupportTest.java. Pester mpetras to implement run-single-method in apisupport.project. [JG07] Mark testDirectNotifyChanges and testDirectNotifyChangesInAtomicAction @RandomlyFails if you are going to use SLEEP.
JL01: what is the inevitable reason why we need PM.mutex() atomic-lock filesystems? This does not seem right to me. To my knowledge, the project does not have to store its metadata on FileSystems. Moreover, it seems to me that we are atomic-locking the FileSystems even for readlocks - why? Wouldn't it be more correct to simply not atomic-lock the FileSystems for PM.mutex()?
the issue seems highly controversial and does not warrant the FAST track review 1. the current "solution" introduces rather incomprehensible concept that will stick with us for a few releases (read forever) 2. the problem was caused by as workaround for a problem that was caused by another controversial problem (DataObject.getLookup(). Personally I don't think the mutex wrapping by atomic action is correct in general and should be rollback. Therefore the current problem disappears (am I right?) and there's no need to introduce this api change.
This introduces an unwanted change in an attempt to defend another unwanted change. It does not contribute to the clarity or ease of use of our API's -- quite the opposite, actually -- so I do no agree with it. I think a better solution would be to remove the AtomicAction wrapping hack. I almost have the list of DataObject's that need to be fixed in order to avoid the deadlocks the AA hack tried to avoid, and I can fix them. Anyway, in case the change is accepted: [AB01] Would too prefer that isMutexEvent() not document the AA hack, since that would make it harder to remove the hack. [AB02] Why is mkubec mentioned as the author of the changes?
Lowering priority to P2, since there are no user visible issue caused by the change so far. We will try to fix it by reverting the runAtomicAction part of the PM.mutex along with other fixes in DataObjects.
Fixed. runAtomicAction part was removed from mutex. http://hg.netbeans.org/main/rev/9839724a6882
Integrated into 'main-golden', will be available in build *200809200201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main/rev/9839724a6882 User: Milan Kubec <mkubec@netbeans.org> Log: #146072: reverting runAtomicAction part of the fix for issue #91291; might cause undefined behavior
Integrated into 'main-golden', will be available in build *200809210201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main/rev/799aec2222cd User: Milan Kubec <mkubec@netbeans.org> Log: #146072: had to revert to previous version of PM.mutex, some tests are failing
Reopening the issue, some tests are failing.
Fixed again, now with tests. http://hg.netbeans.org/main/rev/72931bc4f209
Integrated into 'main-golden', will be available in build *200809231435* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main/rev/72931bc4f209 User: Milan Kubec <mkubec@netbeans.org> Log: #146072: PM.mutex won't run inside runAtomicAction; tests fixed