Sling
  1. Sling
  2. SLING-2222

SlingServerRepository startup and potential NPE

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: JCR Base 2.1.2
    • Component/s: JCR
    • Labels:
      None
    • Environment:
      Eclipse Equinox

      Description

      I'm starting Sling bundles from Equinox and there is now a NPE when starting repository (doesn't seem to happen in launchpad, probably because component and bundle activation order is different there). AbstractNamespaceMappingRepository#namespaceMapperTracker is used before initialized with #setup.

      SlingServerRepository(AbstractNamespaceMappingRepository).defineNamespacePrefixes(Session) line: 65
      SlingServerRepository(AbstractNamespaceMappingRepository).getNamespaceAwareSession(Session) line: 92
      SlingServerRepository(AbstractSlingRepository).login(Credentials, String) line: 216
      SlingServerRepository(AbstractSlingRepository).loginAdministrative(String) line: 171
      SlingServerRepository(AbstractSlingRepository).pingAndCheck() line: 499
      SlingServerRepository(AbstractSlingRepository).startRepository() line: 755
      SlingServerRepository(AbstractSlingRepository).activate(ComponentContext) line: 581

      the above will hit SlingServerRepository before #setup is called. #setup will be called second time around:

      SlingServerRepository(AbstractNamespaceMappingRepository).setup(BundleContext) line: 46
      SlingServerRepository(AbstractSlingRepository).setupRepository(Repository) line: 426
      SlingServerRepository(AbstractSlingRepository).startRepository() line: 758
      SlingServerRepository(AbstractSlingRepository).activate(ComponentContext) line: 581

      I've added NPE check for namespaceMapperTracker in defineNamespacePrefixes which works around this issue. NPE check for other namespaceHandler is already present.

      I would be good to simplify repository pinging code path and avoid the NPE checks though.

        Activity

        Hide
        Justin Edelson added a comment -

        fixed in r1171235.

        BTW, it'd be great if you could post a Wiki page about running Sling on Equinox.

        Show
        Justin Edelson added a comment - fixed in r1171235. BTW, it'd be great if you could post a Wiki page about running Sling on Equinox.
        Hide
        Felix Meschberger added a comment -

        Hmm, I think the AbstarctSlingRepository should actually setup and tear down the AbstractNamespaceMappingRepository from within the activate and deactivate methods and not when the repository is actually set up.

        Show
        Felix Meschberger added a comment - Hmm, I think the AbstarctSlingRepository should actually setup and tear down the AbstractNamespaceMappingRepository from within the activate and deactivate methods and not when the repository is actually set up.
        Hide
        Aleksander Bandelj added a comment -

        I agree, those NPE checks are not smelling good.I think AbstractSlingRepository#setup should be the first thing called in AbstractSlingRepository#activate, not indirectly through #setupRepository in its superclass.

        Show
        Aleksander Bandelj added a comment - I agree, those NPE checks are not smelling good.I think AbstractSlingRepository#setup should be the first thing called in AbstractSlingRepository#activate, not indirectly through #setupRepository in its superclass.
        Hide
        Justin Edelson added a comment -

        I didn't see a good way to do that without an API change which didn't seem warranted. AFAIK, there's a single code path (when the login is attempted from inside the ping check) which can cause this. Plus, even with the API change, the value could still be null. That said, feel free to reopen.

        Show
        Justin Edelson added a comment - I didn't see a good way to do that without an API change which didn't seem warranted. AFAIK, there's a single code path (when the login is attempted from inside the ping check) which can cause this. Plus, even with the API change, the value could still be null. That said, feel free to reopen.
        Hide
        Felix Meschberger added a comment -

        You can rename the two – setup and teardown – methods in the AbstractNamespaceMappingRepository and call them from the AbstractSlingRepository activator ?

        Show
        Felix Meschberger added a comment - You can rename the two – setup and teardown – methods in the AbstractNamespaceMappingRepository and call them from the AbstractSlingRepository activator ?
        Hide
        Justin Edelson added a comment -

        ...which would be an API change.

        Show
        Justin Edelson added a comment - ...which would be an API change.

          People

          • Assignee:
            Justin Edelson
            Reporter:
            Aleksander Bandelj
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development