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 125694 [details] Proposed patch, includes changes in J2SE, WS and FX code, api change, test A year ago we faced the problem of project properties' saving in non-default panels of Project Properties dialog added by J2SE project extensions (WS, FX). The solution accepted then (see issue #200691) allowed each extension to reqister a service (J2SECustomPropertySaver) that would be invoked when OK is pressed in Project Properties dialog. The solution is principally correct but in its original form now appears insufficient. A problem appeared first in form of issues #208019 and #208112; these have been fixed by a hack that has now been shown to lead to a memory leak: issue #219746. The situation to be resolved can be summarized as follows: when Project Properties dialog is open, the J2SE Project extensions need a temporary storage of additional properties whose lifecycle should be bound to the lifecycle of the dialog. However, there is no mechanism for that in the dialog itself which is handled by J2SE project code. Each extension can provide various panels, which may or may not be constructed depending on whether the user opens them or not. Thus the reference to extension's specific properties (if they are constructed) is to be held statically. Such solution is prone to memory leaks though as shown in issue #219746 and should be handled properly. In case the user pressed OK the properties in temporary storage (if it has been created when user opened extension's specific panels) need to be saved. But regardless whether OK or Cancel has been pressed, any temporary storage needs to be eventually erased when the dialog closes. The proposed solution is to replace the original SPI public interface J2SECustomPropertySaver { void save(Project p); } by public interface by J2SECustomPropertyHandler { void save(Project p); void cleanup(Project p); } and modify CustomizerProviderImpl in java.j2seproject module so that it invokes J2SECustomPropertyHandler.save() for each registered project extension on OK press, as has been the case till now with J2SECustomPropertySaver.save(), but also to invoke J2SECustomPropertyHandler.cleanup() for each registered project extension on dialog close. This proposed change will have localized impact and is unlikely to affect any third party code because J2SEProject extension is not a publicly promoted practice. To my knowledge the only two existing extensions are our own javawebstart and javafx2.project modules. The old SPI will be marked deprecated but will remain operational. The change will streamline property handling lifecycle in Project Properties dialog, will prevent memory leaks and will enable to remove some ugly pieces of code.
Would it make sense to define this API in Project UI infrastructure so that it can be used by other project types apart from J2SE?
(In reply to comment #1) > Would it make sense to define this API in Project UI infrastructure so that it > can be used by other project types apart from J2SE? Yes. It would be great if C/C++ can reuse it as well.
If the generic contract is needed I would suggest small change. The problem with the J2SECustomPropertyHandler and the J2SECustomPropertySaver is that they were designed just for J2SE project. The J2SE Project CustomizerProvider is responsible to call them in its store listener. If we make them generic there will be still a need for the CustomizerProvider implementors to call the interface in the store listener, something like: private class StoreListener implements ActionListener { public void actionPerformed(ActionEvent e) { uiProperties.save(); for (J2SECustomPropertySaver saver : project.getLookup().lookupAll(J2SECustomPropertySaver.class)) { saver.save(project); } } } This requires changes in all the affected project types which may even not know about panels which are injected into them. I suggest to move the responsibility to the side of the panel provider. Currently the ProjectCustomizer.Category has setStoreListener and setOKButtonListener methods which are called by the project UI infrastructure (no action needed on the project implementor side). My suggestion is to extend the ProjectCustomizer.Category by setCloseListener(AL) which will be used to do the clean up. The provider of the extension panel will just do the following in the implementation of the ProjectCustomizer.CompositeCategoryProvider.createCategory() method: ProjectCustomizer.Category c = ProjectCustomizer.Category.create(CAT_WEBSTART, NbBundle.getMessage(JWSCompositeCategoryProvider.class, "LBL_Category_WebStart"), null); c.setStoreListener(new ActionListener() { @Override public void actionPerformed(ActionEvent e) { store(); } }); c.setCloseListener(new ActionListener() { @Override public void actionPerformed(ActionEvent e) { free(); } }); return c;
Comment #3 sounds good. I'll react in more detail after the return from vacation.
I tried to implement the suggestion from Comment #3 but hit a serious problem. Close listeners need to be called when the whole properties dialog gets closed; this however happens immediately after the invocations of Store listeners in org.netbeans.modules.project.uiapi.CustomizerDialog.OptionListener.actionPerformed where each store listener is called through RequestProcessor. This is a sync problem, Close listener can effectively destroy data before actually the RequestProcessor thread gets to it (I observed this). Combining the proposed Close listener mechanism with the J2SECustomPropertySaver does not help either - dialog panels apparently get closed (and Close listeners in org.netbeans.modules.project.uiapi.CustomizerDialog called) before j2seproject...CustomizerProviderImpl calls J2SECustomPropertySaver.save(). This does not seem to be a problem in case of the original proposal in this Issue, as it is restricted to use with j2seproject only, where store listeners are not executed in separate threads. Will try to investigate more.
Created attachment 126926 [details] Proposed patch, includes changes in Project UI, J2SE, WS and FX code, api change, test I am attaching a different patch proposal that follows the idea from Comment #3, though due to problems described in Comment #4 its application is less straightforward than expected. Nevertheless it makes the use of J2SECustomPropertySaver services obsolete, what is an advantage in itself. The patch introduces a Close listener to ProjectCustomizer.Category which is called on window close. The usage in WebStart and FX Project dialogs is as follows. Assuming OKButton listeners in ProjectCustomizer.Category are called before Store and Close listeners, and assuming there is no guarantee regarding call order of Store and Close listeners (Store listeners are called asynchronously), the proposed use case is to grab a temporary reference to WS or FX specific properties class in OKButton listener, to do cleanup of all other references to the property class using the Close listener, and to do the actual storage calls in Store listener which also eventually removes the temporary reference.
if there is no more comments I am going to integrate tomorrow.
fixed in jetmain http://hg.netbeans.org/jet-main/rev/569ba1b5a7c2 Note that the changeset also includes changes in WebStart and FX Project modules (JFXProjectProperties and JWSProjectProperties lifecycle and persistence mechanism is changed considerably) in order to fix the memory leak filed as Issue #219746.
Integrated into 'main-golden', will be available in build *201211110001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-golden/rev/569ba1b5a7c2 User: Petr Somol <psomol@netbeans.org> Log: #219827 add Close listener to ProjectCustomizer.Category