Apache S4
  1. Apache S4
  2. S4-74

NullPointerException on TCPEmitter.onChange()

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.5.0
    • Fix Version/s: 0.5.0
    • Labels:
      None

      Description

      TCPEmitter is registering itself as a listener in its constructor: this.topology.addListener(this);

      The topology is triggering a refresh before the TCPEmitter is fully constructed, so TCPEmitter.partitionNodeMap is still null during onChange() execution.

      TCPEmitter shouldn't register itself as a listener until it's fully built, see http://www.ibm.com/developerworks/java/library/j-jtp0618/index.html#2 (Don't publish the "this" reference during construction)

        Activity

        Hide
        Matthieu Morel added a comment -

        Thanks for reporting that.

        I'll add an init method annotated with @Inject and also take the opportunity to call onChange() (renamed to refreshCluster() ) in the initialization, so that the cluster configuration is not discovered upon sending the first event.

        Show
        Matthieu Morel added a comment - Thanks for reporting that. I'll add an init method annotated with @Inject and also take the opportunity to call onChange() (renamed to refreshCluster() ) in the initialization, so that the cluster configuration is not discovered upon sending the first event.
        Hide
        Matthieu Morel added a comment -

        Fixed "this" reference escape during construction in several classes. Updated patch in branch S4-74

        Show
        Matthieu Morel added a comment - Fixed "this" reference escape during construction in several classes. Updated patch in branch S4-74
        Hide
        Matthieu Morel added a comment - - edited
        Show
        Matthieu Morel added a comment - - edited patch updated with a suggestion from Daniel https://git-wip-us.apache.org/repos/asf?p=incubator-s4.git;a=shortlog;h=refs/heads/S4-74
        Hide
        Daniel Gómez Ferro added a comment - - edited

        This fixed the original problem, thanks a lot!

        +1 from my side

        Show
        Daniel Gómez Ferro added a comment - - edited This fixed the original problem, thanks a lot! +1 from my side
        Hide
        Matthieu Morel added a comment -

        Sorry to come back on that one, but I uploaded another commit so that cluster updates are synchronized, as in other parts of the code (here we have potential concurrent updates between Zookeeper callbacks and channel connections). Please let me know if that's ok on your side.

        Show
        Matthieu Morel added a comment - Sorry to come back on that one, but I uploaded another commit so that cluster updates are synchronized, as in other parts of the code (here we have potential concurrent updates between Zookeeper callbacks and channel connections). Please let me know if that's ok on your side.
        Hide
        Daniel Gómez Ferro added a comment -

        Good catch, +1

        Show
        Daniel Gómez Ferro added a comment - Good catch, +1
        Show
        Matthieu Morel added a comment - Merged into piper branch https://git-wip-us.apache.org/repos/asf?p=incubator-s4.git;a=commit;h=4a30483c289f6f92f95f006485634b94b484a524

          People

          • Assignee:
            Matthieu Morel
            Reporter:
            Daniel Gómez Ferro
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development