[GitHub] drill pull request #1079: DRILL-6063: Set correct ThreadContext ClassLoader ...
Github user asfgit closed the pull request at: https://github.com/apache/drill/pull/1079 ---
[GitHub] drill pull request #1079: DRILL-6063: Set correct ThreadContext ClassLoader ...
Github user parthchandra commented on a diff in the pull request: https://github.com/apache/drill/pull/1079#discussion_r159508398 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java --- @@ -344,7 +344,18 @@ private boolean clientNeedsAuthExceptPlain(DrillProperties props) { mechanismName = factory.getSimpleName(); logger.trace("Will try to authenticate to server using {} mechanism with encryption context {}", mechanismName, connection.getEncryptionCtxtString()); + + // Update the thread context class loader to current class loader + // See DRILL-6063 for detailed description + final ClassLoader oldThreadCtxtCL = Thread.currentThread().getContextClassLoader(); + final ClassLoader newThreadCtxtCL = this.getClass().getClassLoader(); + Thread.currentThread().setContextClassLoader(newThreadCtxtCL); + --- End diff -- makes sense ---
[GitHub] drill pull request #1079: DRILL-6063: Set correct ThreadContext ClassLoader ...
Github user sohami commented on a diff in the pull request: https://github.com/apache/drill/pull/1079#discussion_r159500241 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java --- @@ -344,7 +344,18 @@ private boolean clientNeedsAuthExceptPlain(DrillProperties props) { mechanismName = factory.getSimpleName(); logger.trace("Will try to authenticate to server using {} mechanism with encryption context {}", mechanismName, connection.getEncryptionCtxtString()); + + // Update the thread context class loader to current class loader + // See DRILL-6063 for detailed description + final ClassLoader oldThreadCtxtCL = Thread.currentThread().getContextClassLoader(); + final ClassLoader newThreadCtxtCL = this.getClass().getClassLoader(); + Thread.currentThread().setContextClassLoader(newThreadCtxtCL); + --- End diff -- This is the only place since this is when `Hadoop Configuration `object is created inside `createAndLoginUser`. SASL authentication mechanism factories are scanned before this code itself and moreover they are scanned in context of current class ClassLoader not in ThreadContext class loader. ---
[GitHub] drill pull request #1079: DRILL-6063: Set correct ThreadContext ClassLoader ...
Github user parthchandra commented on a diff in the pull request: https://github.com/apache/drill/pull/1079#discussion_r159487621 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java --- @@ -344,7 +344,18 @@ private boolean clientNeedsAuthExceptPlain(DrillProperties props) { mechanismName = factory.getSimpleName(); logger.trace("Will try to authenticate to server using {} mechanism with encryption context {}", mechanismName, connection.getEncryptionCtxtString()); + + // Update the thread context class loader to current class loader + // See DRILL-6063 for detailed description + final ClassLoader oldThreadCtxtCL = Thread.currentThread().getContextClassLoader(); + final ClassLoader newThreadCtxtCL = this.getClass().getClassLoader(); + Thread.currentThread().setContextClassLoader(newThreadCtxtCL); + --- End diff -- Is this the only place we need to update? For eaxmple, with SASL the authentication mechanism plugin would have to be in the class path when the authentication method is dtermined. But you have already restored the thread context. (Or does the oldThreadContextCL have the SASL jars in the classpath?) ---
[GitHub] drill pull request #1079: DRILL-6063: Set correct ThreadContext ClassLoader ...
GitHub user sohami opened a pull request: https://github.com/apache/drill/pull/1079 DRILL-6063: Set correct ThreadContext ClassLoader before using Hadoop⦠⦠Configuration class in DrillClient You can merge this pull request into a Git repository by running: $ git pull https://github.com/sohami/drill DRILL-6063 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/1079.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1079 commit e7a7273df06f0602c6ca154297b2db01a1181c33 Author: Sorabh Hamirwasia Date: 2018-01-02T18:35:19Z DRILL-6063: Set correct ThreadContext ClassLoader before using Hadoop Configuration class in DrillClient ---