Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-5195

Prevent passing null pointer to mlock and munlock.

Details

    • Bug
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • HDFS-4949
    • HDFS-4949
    • datanode
    • None
    • Reviewed

    Description

      According to JNI documentation, it is optional for the JVM to support the GetDirectBufferAddress function. If unsupported, then the function will return null. This is probably a very rare thing, but let's be defensive by checking the return value for null and throwing an exception instead of passing null down to mlock and munlock.

      Attachments

        1. HDFS-5195.1.patch
          2 kB
          Chris Nauroth
        2. HDFS-5195.2.patch
          2 kB
          Chris Nauroth

        Issue Links

          Activity

            cnauroth Chris Nauroth added a comment - Relevant JNI documentation is here: http://docs.oracle.com/javase/6/docs/technotes/guides/jni/spec/functions.html#GetDirectBufferAddress
            cnauroth Chris Nauroth added a comment -

            Here is a patch to check for a null return value. While I was in this area, I also fixed a potential file handle leak in the test code.

            cnauroth Chris Nauroth added a comment - Here is a patch to check for a null return value. While I was in this area, I also fixed a potential file handle leak in the test code.
            cmccabe Colin McCabe added a comment -

            mlock and munlock are able to handle NULL pointers on their own, without any assistance from us. They will simply fail with ENOMEM.

            I do agree that it's helpful to print out a special error message in this case to let the user know that JNI access to direct buffers is not available. However, I would rather not put the check in like you have it here, since it's an extra "if" statement we have to execute all the time, even though 99.9999% of the time it does nothing. Let's add the check after mlock or munlock fails.

            cmccabe Colin McCabe added a comment - mlock and munlock are able to handle NULL pointers on their own, without any assistance from us. They will simply fail with ENOMEM . I do agree that it's helpful to print out a special error message in this case to let the user know that JNI access to direct buffers is not available. However, I would rather not put the check in like you have it here, since it's an extra "if" statement we have to execute all the time, even though 99.9999% of the time it does nothing. Let's add the check after mlock or munlock fails.
            cnauroth Chris Nauroth added a comment -

            Let's add the check after mlock or munlock fails.

            Sounds good. Here is a new patch.

            cnauroth Chris Nauroth added a comment - Let's add the check after mlock or munlock fails. Sounds good. Here is a new patch.
            cmccabe Colin McCabe added a comment -

            +1 for the patch; thanks Chris

            cmccabe Colin McCabe added a comment - +1 for the patch; thanks Chris
            cnauroth Chris Nauroth added a comment -

            I committed this to the HDFS-4949 branch. Thanks for the review, Colin.

            cnauroth Chris Nauroth added a comment - I committed this to the HDFS-4949 branch. Thanks for the review, Colin.

            People

              cnauroth Chris Nauroth
              cnauroth Chris Nauroth
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: