Uploaded image for project: 'Apache S4'
  1. Apache S4
  2. S4-74

NullPointerException on TCPEmitter.onChange()

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: 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
        mmorel 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
        mmorel 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
        mmorel Matthieu Morel added a comment -

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

        Show
        mmorel Matthieu Morel added a comment - Fixed "this" reference escape during construction in several classes. Updated patch in branch S4-74
        Hide
        mmorel Matthieu Morel added a comment - - edited
        Show
        mmorel 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
        dferro Daniel Gómez Ferro added a comment - - edited

        This fixed the original problem, thanks a lot!

        +1 from my side

        Show
        dferro Daniel Gómez Ferro added a comment - - edited This fixed the original problem, thanks a lot! +1 from my side
        Hide
        mmorel 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
        mmorel 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
        dferro Daniel Gómez Ferro added a comment -

        Good catch, +1

        Show
        dferro Daniel Gómez Ferro added a comment - Good catch, +1
        Show
        mmorel 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:
            mmorel Matthieu Morel
            Reporter:
            dferro Daniel Gómez Ferro
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development