glazedlists
  1. glazedlists
  2. GLAZEDLISTS-484

CompositeList.addMemberList doesn't handle reentrancy

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 0.9.1
    • Fix Version/s: milestone 1
    • Component/s: core
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      484

      Description

      If addMemberList is called on CompositeList as part of a list event response,
      the change to the CompositeList is dropped.

      Demonstrating this issue involves a fairly complex list pipeline. I've prepared
      a unit test that demonstrates the issue.

      As near as I can tell, the dropped event occurs because of two things in
      SequenceDependenciesEventPublisher.fireEvent():

      1. the loop that checks for subjects (the first for loop in the method) does
      not find any entries for the addMemberList event (maybe a listener isn't getting
      registered properly??)
      2. reentrantFireEventCount is equal to 2 during the addMemberList event
      handler, so processing delegates to the stack (but because of #1, nothing gets
      added for the stack to handle)

        Activity

        Hide
        trumpetinc added a comment -

        Created an attachment (id=50)
        Unit test demonstrating issue

        Show
        trumpetinc added a comment - Created an attachment (id=50) Unit test demonstrating issue
        Hide
        trumpetinc added a comment -

        I thought I should mention that the unit test is contrived to show the problem
        with the minimal pipeline that I could come up with - obviously, there are more
        effective pipelines for implementing the behavior that the unit test is trying
        for. I just wanted to make sure that this simplification wasn't causing the
        issue to be ignored.

        Show
        trumpetinc added a comment - I thought I should mention that the unit test is contrived to show the problem with the minimal pipeline that I could come up with - obviously, there are more effective pipelines for implementing the behavior that the unit test is trying for. I just wanted to make sure that this simplification wasn't causing the issue to be ignored.
        Hide
        brands added a comment -

        I think the problem is the following:

        All lists in your test case use the same ListEventPublisher.
        When a top-level event is fired in the publisher (reentrantFireEventCount == 0),
        it takes a snapshot of the current SubjectAndListener list.
        From that list, it determines the listeners to notify.
        If those listeners in turn establish new Subject/Listener relationships with the
        same publisher, they take no effect for the currently active event processing
        (including the recursive processing caused by the top-level event
        (reentrantFireEventCount > 0)).

        The updated SubjectAndListener list is considered only when a new top-level
        event processing starts.

        So I think it's not directly related to CompositeList/CollectionList, but a more
        general problem.

        Jesse/James, are these statements correct and do you have an idea how to resolve
        or workaround this issue?

        Show
        brands added a comment - I think the problem is the following: All lists in your test case use the same ListEventPublisher. When a top-level event is fired in the publisher (reentrantFireEventCount == 0), it takes a snapshot of the current SubjectAndListener list. From that list, it determines the listeners to notify. If those listeners in turn establish new Subject/Listener relationships with the same publisher, they take no effect for the currently active event processing (including the recursive processing caused by the top-level event (reentrantFireEventCount > 0)). The updated SubjectAndListener list is considered only when a new top-level event processing starts. So I think it's not directly related to CompositeList/CollectionList, but a more general problem. Jesse/James, are these statements correct and do you have an idea how to resolve or workaround this issue?

          People

          • Assignee:
            jessewilson
            Reporter:
            trumpetinc
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated: