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: | DataObject registration in the DataObjectPool is done on partially constructed DataObjects | ||
---|---|---|---|
Product: | platform | Reporter: | Chris Webster <cwebster> |
Component: | -- Other -- | Assignee: | Jaroslav Tulach <jtulach> |
Status: | CLOSED FIXED | ||
Severity: | blocker | CC: | asherman, jcortopassi, non_migrated_user |
Priority: | P2 | ||
Version: | 3.x | ||
Hardware: | Sun | ||
OS: | Solaris | ||
Issue Type: | DEFECT | Exception Reporter: | |
Bug Depends on: | |||
Bug Blocks: | 20022 | ||
Attachments: |
diff against version 1.45 of MultiDataObject.java
Diff against version 1.33.2.3.6.1 of DataObjectPool.java Patch with sources and classes that runs in release32 |
Description
Chris Webster
2001-06-14 17:34:11 UTC
1. Did the suggested change in synchronization helped you? Should we apply it? 2. "The final test to determine if the object exists is performed in the constructor" - the test is performed before the constructor, because the method DataObjectPool.POOL.register is static. As far as I know the object is not allocated yet. 3. "exceptions, namely DataObjectExistsException are being used" - true. But it is hard to do anything with it and keep backward compatibility. I'll apply the change in synchronization in main trunk. Let me know whether and in which branches it should be applied too. Created attachment 1600 [details]
diff against version 1.45 of MultiDataObject.java
Created attachment 1601 [details]
Diff against version 1.33.2.3.6.1 of DataObjectPool.java
Synchronizing the access to toNotify in DataObjectPool.find does not fix the problem. It is really just a side issue. The main problem is that since the call to DataObjectPool.register is done in DataObject's constructor, the registration finishes before the data object's subclass constructors have run and thus a partially constructed data object is in the pool's table and available for another thread to find and use. It seems like any potential fix can not complete the registration until the newly created data object is fully constructed. Since we probably can't change the protocol used to create data objects, it is not clear how to do this unless you know every place that creates a new data object. We did play with a partial fix that seemed to work for out test. In DataObjectPool.register we mark the Item for the new data object as not initialized. Other places in DataObjectPool that do "get" on the map synchronize on the found Item and if it is not initialized, then they wait on the found Item. callHandleFind marks the Item for the found data object as initialized and then does a notifyAll on the found Item. Thus the other places in DataObjectPool won't ever return an uninitialized data object. Of course this requires other places that create a data object to somehow get the data object's Item marked as initialized. We changed MultiDataObject.handleCreateFromTemplate to call a new method in DataObjectPool to deal with this, but this would have to been done at any place a DataObject is created, and I have no idea whether that is pratical (for example are data objects deserialized?). I'm including the changed files as an attachment but I need to say again, I don't consider this a fix since I have no idea all the ways and places a DataObject may be created. Hopefully someone who understands all the ways DataObject can be created can figure out how to get something like this working. Additional comments and changes posted,problem still remains after fix. I do not want to add new field into DataObjectPool.Item to save a bit of memory. I'll try to enhance the toNotify mechanism which was originaly designed to prevent the problems described in this bug, but does not seem to be correct. Fixed. DataObjectPool rev. 1.41 and DataLoaderPool rev. 1.67 I've keep the toNotify mechanism, but to new approach is to wait at least 200ms before cleaning the toNotify HashSet. More details: If a data object is created by DataLoaderPool.findDataObject or by DataObject.find, immediatelly after the object is created a DataObjectPool.notifyCreated (dataObject) method is called, object is removed from toNotify and if toNotify is empty, it is set to null. (by toNotify == null, one can use the DataObjectPool.find (FileObject) that caused the problems that lead to this bug). If a data object is created in some other way - for example by invoking its constructor directly. It is added into toNotify set and a task is started to later notify its creation. After 200ms of no changes in toNotify set, all its objects are expected to finish their constructors and are notified. The second case is just fallback case for strange situations when one is not using DataObject.find, but should guarantee that creation of all objects is notified to listeners sometimes. Can this fix be added to 3.2.1 Chris, can you verify this issue? Thanks. If you will this have in 3.2.1, add keyword 3.2.1_CANDIDATE, the issue have to be verified and reviewed. Would you verification be valid by using only the new versions of MultiDataObject and DataObjectPool with the rest of the source from the pilsen_fcs branch? The verification will be valid when using DataObjectPool rev. 1.41 and DataLoaderPool rev. 1.67. When I update DataObjectPool and DataLoaderPool cvs -r update -r 1.41 DataObjectPool.java cvs -r update -r 1.67 DataLoaderPool.java the following compilation errors result when compiling against the pilsen_fcs branch: It seems as though other files have changed as well since the pilsen_fcs branch. Any ideas compile-regular: [javac] Compiling 2 source files to /bling1/home/cwebster/cvs/nb_all/openide/src [javac] /bling1/home/cwebster/cvs/nb_all/openide/src/org/openide/loaders/DataLoaderPool.java:472: cannot resolve symbol [javac] symbol : constructor UniFileLoader (java.lang.String) [javac] location: class org.openide.loaders.UniFileLoader [javac] super ("org.openide.loaders.DataFolder"); [javac] ^ [javac] /bling1/home/cwebster/cvs/nb_all/openide/src/org/openide/loaders/DataLoaderPool.java:526: cannot resolve symbol [javac] symbol : class Folder [javac] location: class org.openide.loaders.FileEntry [javac] return new FileEntry.Folder(obj, primaryFile); [javac] ^ [javac] /bling1/home/cwebster/cvs/nb_all/openide/src/org/openide/loaders/DataLoaderPool.java:530: incompatible types [javac] found : org.openide.loaders.DataFolder [javac] required: org.openide.loaders.MultiDataObject [javac] return new DataFolder (primaryFile); [javac] ^ [javac] /bling1/home/cwebster/cvs/nb_all/openide/src/org/openide/loaders/DataLoaderPool.java:551: cannot resolve symbol [javac] symbol : constructor UniFileLoader (java.lang.String) [javac] location: class org.openide.loaders.UniFileLoader [javac] super ("org.openide.loaders.InstanceDataObject"); [javac] ^ [javac] /bling1/home/cwebster/cvs/nb_all/openide/src/org/openide/loaders/DataLoaderPool.java:646: cannot resolve symbol [javac] symbol : constructor MultiFileLoader (java.lang.String) [javac] location: class org.openide.loaders.MultiFileLoader [javac] super ("org.openide.loaders.DefaultDataObject"); [javac] ^ [javac] /bling1/home/cwebster/cvs/nb_all/openide/src/org/openide/loaders/DataLoaderPool.java:749: cannot resolve symbol [javac] symbol : constructor UniFileLoader (java.lang.String) [javac] location: class org.openide.loaders.UniFileLoader [javac] super ("org.openide.loaders.DataShadow"); [javac] ^ [javac] /bling1/home/cwebster/cvs/nb_all/openide/src/org/openide/loaders/DataLoaderPool.java:797: incompatible types [javac] found : org.openide.loaders.DataShadow [javac] required: org.openide.loaders.MultiDataObject [javac] if (d != null) return new DataShadow (primaryFile, d, this); [javac] ^ [javac] /bling1/home/cwebster/cvs/nb_all/openide/src/org/openide/loaders/DataLoaderPool.java:802: incompatible types [javac] found : org.openide.loaders.BrokenDataShadow [javac] required: org.openide.loaders.MultiDataObject [javac] return new BrokenDataShadow (primaryFile, this); [javac] ^ [javac] /bling1/home/cwebster/cvs/nb_all/openide/src/org/openide/loaders/DataObjectPool.java:119: cannot resolve symbol [javac] symbol : method changedDataSystem (org.openide.filesystems.FileObject) [javac] location: class org.openide.loaders.FolderList [javac] FolderList.changedDataSystem (file); [javac] ^ [javac] /bling1/home/cwebster/cvs/nb_all/openide/src/org/openide/loaders/DataObjectPool.java:630: cannot resolve symbol [javac] symbol : method changedDataSystem (org.openide.filesystems.FileObject) [javac] location: class org.openide.loaders.FolderList [javac] FolderList.changedDataSystem (df.getPrimaryFile ()); [javac] ^ [javac] 10 errors BUILD FAILED /bling1/home/cwebster/cvs/nb_all/openide/build.xml:44: Compile failed, messages should have been provided. You are right, Chris, it is not as easy. There were more changes in the main trunk lately. Created attachment 1743 [details]
Patch with sources and classes that runs in release32
Please evaluate and add keyword for release32 if desired. Otherwise seems to be fixed. The regression test detecting the problem was run with the new change, but the change did not resolve the problem. Therefore, I do not recommend integration into NB 3.2.1 Resolved for 3.4.x or earlier, no new info since then -> verified Resolved for 3.4.x or earlier, no new info since then -> closing. |