Hadoop Common
  1. Hadoop Common
  2. HADOOP-9212

Potential deadlock in FileSystem.Cache/IPC/UGI

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.2-alpha
    • Fix Version/s: 2.0.3-alpha, 0.23.7
    • Component/s: fs
    • Labels:
      None

      Description

      jcarder found a cycle which could lead to a potential deadlock.

      1. 1_jcarder_result_0.png
        54 kB
        Tom White
      2. HADOOP-9212.patch
        3 kB
        Tom White
      3. HADOOP-9212.patch
        3 kB
        Tom White

        Activity

        Hide
        Tom White added a comment -

        The scenario that leads to the potential deadlock:

        • FS.Cache.closeAll(), which is holding the FS.Cache lock, calls DFS's close method which calls close on the RPC proxy, which eventually calls ipc.Client.stop() and takes lock on Hashtable of connections
        • ipc.Client.getConnection(), which is holding lock on Hashtable of connections, takes lock on UGI's class during some UGI setup that trigger's UGI.ensureInitialized()
        • UGI.getCurrentUser(), which is holding UGI class lock, calls getLoginUser() which calls Credentials.readTokenStorageFile, which uses FileSystem, taking a lock on FileSystem.Cache
        Show
        Tom White added a comment - The scenario that leads to the potential deadlock: FS.Cache.closeAll(), which is holding the FS.Cache lock, calls DFS's close method which calls close on the RPC proxy, which eventually calls ipc.Client.stop() and takes lock on Hashtable of connections ipc.Client.getConnection(), which is holding lock on Hashtable of connections, takes lock on UGI's class during some UGI setup that trigger's UGI.ensureInitialized() UGI.getCurrentUser(), which is holding UGI class lock, calls getLoginUser() which calls Credentials.readTokenStorageFile, which uses FileSystem, taking a lock on FileSystem.Cache
        Hide
        Tom White added a comment -

        Here's a fix to break the cycle by using the Java File API in Credentials.readTokenStorageFile (overloading the method to take File) so the FileSystem.Cache lock need not be taken.

        Show
        Tom White added a comment - Here's a fix to break the cycle by using the Java File API in Credentials.readTokenStorageFile (overloading the method to take File) so the FileSystem.Cache lock need not be taken.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12564909/HADOOP-9212.patch
        against trunk revision .

        +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 javac. The applied patch does not increase the total number of javac compiler warnings.

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

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        -1 release audit. The applied patch generated 2 release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common.

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

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2045//testReport/
        Release audit warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/2045//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2045//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12564909/HADOOP-9212.patch against trunk revision . +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 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. -1 release audit . The applied patch generated 2 release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2045//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/2045//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2045//console This message is automatically generated.
        Hide
        Todd Lipcon added a comment -
        • the cleanup of the 'in' stream should be in a finally clause, rather than in the catch clause (in case there's some non-IOE thrown)
        • do you need buffering on the input stream?
        • Can you add a reference to this JIRA in the comment change in UserGroupInformation.java? Otherwise I don't think anyone will know about this lock cycle in a couple months when we've forgotten about this JIRA.
        Show
        Todd Lipcon added a comment - the cleanup of the 'in' stream should be in a finally clause, rather than in the catch clause (in case there's some non-IOE thrown) do you need buffering on the input stream? Can you add a reference to this JIRA in the comment change in UserGroupInformation.java? Otherwise I don't think anyone will know about this lock cycle in a couple months when we've forgotten about this JIRA.
        Hide
        Tom White added a comment -

        Thanks for the review Todd. Here's a new patch with your feedback addressed.

        Show
        Tom White added a comment - Thanks for the review Todd. Here's a new patch with your feedback addressed.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12564991/HADOOP-9212.patch
        against trunk revision .

        +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 javac. The applied patch does not increase the total number of javac compiler warnings.

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

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

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

        +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common.

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

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2049//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2049//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12564991/HADOOP-9212.patch against trunk revision . +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 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2049//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2049//console This message is automatically generated.
        Hide
        Todd Lipcon added a comment -

        +1, looks good to me. Nice analysis of the cycle.

        Show
        Todd Lipcon added a comment - +1, looks good to me. Nice analysis of the cycle.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-trunk-Commit #3249 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3249/)
        HADOOP-9212. Potential deadlock in FileSystem.Cache/IPC/UGI. (Revision 1433879)

        Result = SUCCESS
        tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433879
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Credentials.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
        Show
        Hudson added a comment - Integrated in Hadoop-trunk-Commit #3249 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3249/ ) HADOOP-9212 . Potential deadlock in FileSystem.Cache/IPC/UGI. (Revision 1433879) Result = SUCCESS tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433879 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Credentials.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Yarn-trunk #98 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/98/)
        HADOOP-9212. Potential deadlock in FileSystem.Cache/IPC/UGI. (Revision 1433879)

        Result = SUCCESS
        tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433879
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Credentials.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
        Show
        Hudson added a comment - Integrated in Hadoop-Yarn-trunk #98 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/98/ ) HADOOP-9212 . Potential deadlock in FileSystem.Cache/IPC/UGI. (Revision 1433879) Result = SUCCESS tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433879 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Credentials.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
        Hide
        Tom White added a comment -

        I just committed this.

        Show
        Tom White added a comment - I just committed this.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1287 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1287/)
        HADOOP-9212. Potential deadlock in FileSystem.Cache/IPC/UGI. (Revision 1433879)

        Result = FAILURE
        tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433879
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Credentials.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1287 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1287/ ) HADOOP-9212 . Potential deadlock in FileSystem.Cache/IPC/UGI. (Revision 1433879) Result = FAILURE tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433879 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Credentials.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1315 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1315/)
        HADOOP-9212. Potential deadlock in FileSystem.Cache/IPC/UGI. (Revision 1433879)

        Result = FAILURE
        tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433879
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Credentials.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1315 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1315/ ) HADOOP-9212 . Potential deadlock in FileSystem.Cache/IPC/UGI. (Revision 1433879) Result = FAILURE tomwhite : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433879 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Credentials.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
        Hide
        Thomas Graves added a comment -

        merged to branch-0.23

        Show
        Thomas Graves added a comment - merged to branch-0.23
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-0.23-Build #498 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/498/)
        HADOOP-9212. Potential deadlock in FileSystem.Cache/IPC/UGI (Tom White via tgraves) (Revision 1434880)

        Result = FAILURE
        tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1434880
        Files :

        • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Credentials.java
        • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #498 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/498/ ) HADOOP-9212 . Potential deadlock in FileSystem.Cache/IPC/UGI (Tom White via tgraves) (Revision 1434880) Result = FAILURE tgraves : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1434880 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Credentials.java /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java

          People

          • Assignee:
            Tom White
            Reporter:
            Tom White
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development