Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-6554

DelegationTokenSecretManager lifecycle is inconsistent

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Duplicate
    • Affects Version/s: 0.22.0
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      FSNamesystem.dtSecretManager is initialized in FSNamesystem.initialize(), but not started until activate(). In FSN.stop() it calls dtSecretManager.stopThreads so long as it's non-null, but doesn't check that the threads have started (ie activate() may never have been called). This causes an NPE in stopThreads()

      To fix this, stopThreads should check tokenRemoverThread against null before interrupting it.

      1. hadoop-6554.txt
        1 kB
        Todd Lipcon
      2. hadoop-6554.txt
        1 kB
        Todd Lipcon
      3. hdfs-967.txt
        0.7 kB
        Todd Lipcon

        Issue Links

          Activity

          Hide
          tomwhite Tom White added a comment -

          This was fixed in HADOOP-6573.

          Show
          tomwhite Tom White added a comment - This was fixed in HADOOP-6573 .
          Hide
          tlipcon Todd Lipcon added a comment -

          Can someone please commit this trivial fix?

          Show
          tlipcon Todd Lipcon added a comment - Can someone please commit this trivial fix?
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12435927/hadoop-6554.txt
          against trunk revision 910169.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/363/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/363/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/363/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/363/console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12435927/hadoop-6554.txt against trunk revision 910169. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/363/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/363/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/363/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/363/console This message is automatically generated.
          Hide
          tlipcon Todd Lipcon added a comment -

          changed to a null check instead of checking the running boolean as Steve suggested

          not adding a unit test - this is just a trivial fix that's obvious and not likely to regress. Plus, if it causes a problem in "real life" it should be exposed by other functional level tests, imo.

          Show
          tlipcon Todd Lipcon added a comment - changed to a null check instead of checking the running boolean as Steve suggested not adding a unit test - this is just a trivial fix that's obvious and not likely to regress. Plus, if it causes a problem in "real life" it should be exposed by other functional level tests, imo.
          Hide
          steve_l Steve Loughran added a comment -
          1. I think the check should maybe include one for null-ness, not just is-running. Just to be sure
          2. I can envisage a fairly straightforward test for this problem too
          3. any call to System.exit() is bad and will only get picked up and rejected by a security manager. Best to make handing explicit -and have a separate bugrep for it.
          Show
          steve_l Steve Loughran added a comment - I think the check should maybe include one for null-ness, not just is-running. Just to be sure I can envisage a fairly straightforward test for this problem too any call to System.exit() is bad and will only get picked up and rejected by a security manager. Best to make handing explicit -and have a separate bugrep for it.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12435483/hadoop-6554.txt
          against trunk revision 908353.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/352/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/352/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/352/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/352/console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12435483/hadoop-6554.txt against trunk revision 908353. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/352/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/352/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/352/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/352/console This message is automatically generated.
          Hide
          tomwhite Tom White added a comment -

          Sort of related: I'm not sure of the wisdom of exiting the VM in ExpiredTokenRemover when a Throwable is caught. Would it not be better for the client of AbstractDelegationTokenSecretManager to determine what action to take in this case?

          Show
          tomwhite Tom White added a comment - Sort of related: I'm not sure of the wisdom of exiting the VM in ExpiredTokenRemover when a Throwable is caught. Would it not be better for the client of AbstractDelegationTokenSecretManager to determine what action to take in this case?
          Hide
          tlipcon Todd Lipcon added a comment -

          Oops, this got moved to common this morning. This patch no longer applies.

          Show
          tlipcon Todd Lipcon added a comment - Oops, this got moved to common this morning. This patch no longer applies.

            People

            • Assignee:
              tlipcon Todd Lipcon
              Reporter:
              tlipcon Todd Lipcon
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development