swingx
  1. swingx
  2. SWINGX-746

NO_OP_SELECTION_MANAGER prevents GC on JXTreeTable

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.9.0
    • Fix Version/s: 0.9.2
    • Component/s: TreeTable
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      746

      Description

      NO_OP_SELECTION_MANAGER, currently defined as a private static, will be
      referenced by the class loader. This results in a reference chain connecting
      the classloader and the JXTreeTable. Therefore the table is never garbage
      collected, and if it is in a component hierarchy, the entire hierarchy cannot be
      gc'd.

      Recommended fix: Revert to prior implementation of getSelectionMapper().
      Jeanette says a previous revision used getSelectionMapper() as a lazy
      initializer for the member field and did not define the field as static,
      removing the connection to the classloader and also preventing NPE during
      JXTable constructors.

        Activity

        Hide
        kleopatra added a comment -

        reverted to lazy instantiation, should be okay now (as of build #828).

        Please let me know if there are still problems

        Jeanette

        Show
        kleopatra added a comment - reverted to lazy instantiation, should be okay now (as of build #828). Please let me know if there are still problems Jeanette
        Hide
        kleopatra added a comment -

        Jesse,

        CC'ed just in case you have a similar construct somewhere in glazedlists

        CU
        Jeanette

        Show
        kleopatra added a comment - Jesse, CC'ed just in case you have a similar construct somewhere in glazedlists CU Jeanette
        Hide
        jessewilson added a comment -

        Thanks for the heads up.

        This seems like it's really a bug in the JXTable, for not unregistering itself as a listener to the selection
        manager? I doubt the NO_OP_SELECTION_MANAGER has much control over what it can or cannot
        reference.

        Show
        jessewilson added a comment - Thanks for the heads up. This seems like it's really a bug in the JXTable, for not unregistering itself as a listener to the selection manager? I doubt the NO_OP_SELECTION_MANAGER has much control over what it can or cannot reference.
        Hide
        kleopatra added a comment -

        Hi Jesse,

        Hmm ... no, I think it's that static reference: the issue is that no-op-mapper
        keeps a reference to the treetable's selectionModel. Which is rather creepy in
        itself, the one no-op-mapper returns the selectionModel of the last intantiated
        JXTreeTable or which was last set. That's bound to make trouble.

        On the other hand, that should keep the memory leak to a single JXTreeTable
        instance, haha. Or it's too early in the day for me <g>

        Anyway, replacing the static by a lazily created no-op solves the leak (hope so,
        didn't hear from Carl yet).

        CU

        Show
        kleopatra added a comment - Hi Jesse, Hmm ... no, I think it's that static reference: the issue is that no-op-mapper keeps a reference to the treetable's selectionModel. Which is rather creepy in itself, the one no-op-mapper returns the selectionModel of the last intantiated JXTreeTable or which was last set. That's bound to make trouble. On the other hand, that should keep the memory leak to a single JXTreeTable instance, haha. Or it's too early in the day for me <g> Anyway, replacing the static by a lazily created no-op solves the leak (hope so, didn't hear from Carl yet). CU
        Hide
        carlfreeland added a comment -

        We don't use the CVS version at work, so I couldn't just update and test.
        Probably early next week I'll grab the diffs and apply them to our source.

        Thanks!

        -Carl

        Show
        carlfreeland added a comment - We don't use the CVS version at work, so I couldn't just update and test. Probably early next week I'll grab the diffs and apply them to our source. Thanks! -Carl

          People

          • Assignee:
            kleopatra
            Reporter:
            carlfreeland
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: