Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.2
    • Fix Version/s: 1.2 Beta
    • Component/s: core
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      103

      Description

      Hi Karsten et al.,

      to avoid memory leaks we need to remove listeners from our long-lived beans.
      For example, BeanAdapter provides a release() method to accomplish this *without
      side effects*. I'm missing such a functionality in PresentationModel.
      I know, the recommodation is to call PresentationModel.setBean(null), but this
      method has side effects.
      We pass our own ValueModel as BeanChannel into the PresentationModel and want to
      access the potentially changed bean after closing the dialog. But when closing
      the dialog we want to dispose the associated PresentationModel and calling
      setBean(null) at this point would be to early.
      I hope you see the problem...

      So my suggestion is to either
      a) provide a PresentationModel.release() method which removes all listeners
      without side effects
      – or –
      b) make the beanAdapter accessable for subclasses of PresentationModel, so we
      can call beanAdapter.release() ourselves.

      Does this make sense to you?

      Thanks,
      Holger

        Activity

        Hide
        karsten added a comment -

        Added PresentationModel#release. Will be included in the 1.2 beta.

        Show
        karsten added a comment - Added PresentationModel#release. Will be included in the 1.2 beta.
        Hide
        brands added a comment -

        I just tested the 1.2 RC.
        I'm reopening this bug because of the following issue:

        There is a problem when PresentationModel.release() is called more than one time
        on the same instance during the dispose process.
        Consider the following scenario:

        A multipage dialog (wizard for example) for editing values of a java bean. The
        bean and the associated PresentationModel is shared between the pages. When the
        dialog is disposed, the pages are disposed as a consequence. This can lead to
        multiple calls of PresentationModel.release() on the same instance.
        In these cases I get the exception below.
        The reason is, that in the second call to release() the member
        'propertyChangeHandler' is already 'null' which leads to the NPE.

        So the problem here is the assumption that calls to
        addChangeHandlerTo and removeChangeHandlerFrom
        are always alternating.
        This is not the case anymore, because you cannot always control how often
        PresentationModel.release() is called when disposing dialogs.
        (For example, dialogs with another dialog as parent get disposed (again), when
        the parent dialog is disposed...)

        Here is the exception trace:

        java.lang.NullPointerException: The listener must not be null.
        at
        com.jgoodies.binding.beans.BeanUtils.removePropertyChangeListener(BeanUtils.java:460)
        at
        com.jgoodies.binding.beans.BeanAdapter.removeChangeHandlerFrom(BeanAdapter.java:1071)
        at com.jgoodies.binding.beans.BeanAdapter.release(BeanAdapter.java:971)
        at com.jgoodies.binding.PresentationModel.release(PresentationModel.java:1520)
        at
        de.eurodata.validation.swing.AbstractValidatingPresentationModel.dispose(AbstractValidatingPresentationModel.java:139)
        at
        de.eurodata.swing.JValidatableDialogPage.dispose(JValidatableDialogPage.java:146)
        at de.eurodata.swing.MultiPageModel.disposePageList(MultiPageModel.java:120)
        at de.eurodata.swing.MultiPageModel.dispose(MultiPageModel.java:131)
        at de.eurodata.swing.JPropertyDialog.disposePageModel(JPropertyDialog.java:356)
        at de.eurodata.swing.JPropertyDialog.dispose(JPropertyDialog.java:367)

        Show
        brands added a comment - I just tested the 1.2 RC. I'm reopening this bug because of the following issue: There is a problem when PresentationModel.release() is called more than one time on the same instance during the dispose process. Consider the following scenario: A multipage dialog (wizard for example) for editing values of a java bean. The bean and the associated PresentationModel is shared between the pages. When the dialog is disposed, the pages are disposed as a consequence. This can lead to multiple calls of PresentationModel.release() on the same instance. In these cases I get the exception below. The reason is, that in the second call to release() the member 'propertyChangeHandler' is already 'null' which leads to the NPE. So the problem here is the assumption that calls to addChangeHandlerTo and removeChangeHandlerFrom are always alternating. This is not the case anymore, because you cannot always control how often PresentationModel.release() is called when disposing dialogs. (For example, dialogs with another dialog as parent get disposed (again), when the parent dialog is disposed...) Here is the exception trace: java.lang.NullPointerException: The listener must not be null. at com.jgoodies.binding.beans.BeanUtils.removePropertyChangeListener(BeanUtils.java:460) at com.jgoodies.binding.beans.BeanAdapter.removeChangeHandlerFrom(BeanAdapter.java:1071) at com.jgoodies.binding.beans.BeanAdapter.release(BeanAdapter.java:971) at com.jgoodies.binding.PresentationModel.release(PresentationModel.java:1520) at de.eurodata.validation.swing.AbstractValidatingPresentationModel.dispose(AbstractValidatingPresentationModel.java:139) at de.eurodata.swing.JValidatableDialogPage.dispose(JValidatableDialogPage.java:146) at de.eurodata.swing.MultiPageModel.disposePageList(MultiPageModel.java:120) at de.eurodata.swing.MultiPageModel.dispose(MultiPageModel.java:131) at de.eurodata.swing.JPropertyDialog.disposePageModel(JPropertyDialog.java:356) at de.eurodata.swing.JPropertyDialog.dispose(JPropertyDialog.java:367)
        Hide
        karsten added a comment -

        This issue is fixed as the missing #release method has been added. And I won't
        change the release semantics for the existing #release methods in the 1.2 frame.
        The API docs of the existing #release methods as well as for this new method
        specify, that the release target must not be used anymore, once #release has
        been called.

        You can ensure that you call #release once only from outside.

        In your situation, I'd set the bean to null using #setBean(null) and would
        change the bean channel before to a ValueHolder. This way you won't affect the
        value of your custom bean channel that holds the edited bean.

        Show
        karsten added a comment - This issue is fixed as the missing #release method has been added. And I won't change the release semantics for the existing #release methods in the 1.2 frame. The API docs of the existing #release methods as well as for this new method specify, that the release target must not be used anymore, once #release has been called. You can ensure that you call #release once only from outside. In your situation, I'd set the bean to null using #setBean(null) and would change the bean channel before to a ValueHolder. This way you won't affect the value of your custom bean channel that holds the edited bean.
        Hide
        brands added a comment -

        At least you could consider to make the implementation of
        BeanAdapter.removeChangeHandlerFrom(...) more robust:

        private void removeChangeHandlerFrom(Object bean)

        { // NEW: the null check of propertyChangeHandler if (!observeChanges || bean == null || propertyChangeHandler == null) return; BeanUtils.removePropertyChangeListener(bean, getBeanClass(bean), propertyChangeHandler); propertyChangeHandler = null; }

        That does not change any significant semantics and would solve my problem.
        Thanks,
        Holger

        Show
        brands added a comment - At least you could consider to make the implementation of BeanAdapter.removeChangeHandlerFrom(...) more robust: private void removeChangeHandlerFrom(Object bean) { // NEW: the null check of propertyChangeHandler if (!observeChanges || bean == null || propertyChangeHandler == null) return; BeanUtils.removePropertyChangeListener(bean, getBeanClass(bean), propertyChangeHandler); propertyChangeHandler = null; } That does not change any significant semantics and would solve my problem. Thanks, Holger

          People

          • Assignee:
            binding-issues
            Reporter:
            brands
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: