[
https://issues.apache.org/jira/browse/HADOOP-9232?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13564593#comment-13564593
]
Chris Nauroth commented on HADOOP-9232:
---------------------------------------
Thanks, Ivan. I applied the patch locally and tested a few HDFS operations and
MapReduce jobs. I didn't need to override the config to
{{ShellBasedUnixGroupsMapping}}. It worked great! I also did a build with
-Pnative in an Ubuntu VM to confirm that it didn't accidentally harm native
compilation on Linux.
Here are a few questions:
1. Regarding {{throw_ioe}}, it looks like an almost-copy of the #ifdef WINDOWS
path of the function in {{NativeIO.c}}. Can we refactor and reuse the same
{{throw_ioe}} everywhere, or is that too cumbersome?
2. Assuming that the answer to #1 is that we really need to keep a separate
{{throw_ioe}} in here, then is it intentional that this version uses
LPSTR/FormatMessageA, whereas the version in {{NativeIO.c}} uses
LPWSTR/FormatMessageW?
{code}
...
LPSTR buffer = NULL;
const char* message = NULL;
len = FormatMessageA(
...
{code}
3. Once again assuming that we need to keep the separate {{throw_ioe}}, I don't
think we need to NULL out buffer before returning. For the version in
{{NativeIO.c}}, this was required to prevent a double-free later in the control
flow, but this version only has one possible path to calling {{LocalFree}}.
{code}
...
LocalFree(buffer);
buffer = NULL;
return;
}
{code}
4. Is the following code not thread-safe?
{code}
...
static jobjectArray emptyGroups = NULL;
...
if (emptyGroups == NULL) {
jobjectArray lEmptyGroups = (jobjectArray)(*env)->NewObjectArray(env, 0,
(*env)->FindClass(env, "java/lang/String"), NULL);
if (lEmptyGroups == NULL) {
goto cleanup;
}
emptyGroups = (*env)->NewGlobalRef(env, lEmptyGroups);
if (emptyGroups == NULL) {
goto cleanup;
}
}
{code}
For example, assume 2 threads concurrently call {{getGroupForUser}}. Thread 1
executes the NULL check, enters the if body, and then gets suspended by the OS.
Thread 2 executes and {{emptyGroups}} is still NULL, so it initializes it.
Then, the OS resumes thread 1, which proceeds inside the if body and calls
NewObjectArray again. Since {{emptyGroups}} never gets freed, I believe the
net effect would be a small memory leak.
5. On the {{THROW}} calls, can we add strings that describe the point of
failure (i.e. "Couldn't allocate memory for user") instead of NULL for the
third argument?
{code}
user = (*env)->GetStringChars(env, juser, NULL);
if (user == NULL) {
THROW(env, "java/lang/OutOfMemoryError", NULL);
goto cleanup;
}
{code}
Thanks, again!
> JniBasedUnixGroupsMappingWithFallback fails on Windows with
> UnsatisfiedLinkError
> --------------------------------------------------------------------------------
>
> Key: HADOOP-9232
> URL: https://issues.apache.org/jira/browse/HADOOP-9232
> Project: Hadoop Common
> Issue Type: Bug
> Components: native, security
> Affects Versions: trunk-win
> Reporter: Chris Nauroth
> Assignee: Ivan Mitic
> Attachments: HADOOP-9232.branch-trunk-win.jnigroups.2.patch,
> HADOOP-9232.branch-trunk-win.jnigroups.patch
>
>
> {{JniBasedUnixGroupsMapping}} calls native code which isn't implemented
> properly for Windows, causing {{UnsatisfiedLinkError}}. The fallback logic
> in {{JniBasedUnixGroupsMappingWithFallback}} works by checking if the native
> code is loaded during startup. In this case, hadoop.dll is present and
> loaded, but it doesn't contain the right code. There will be no attempt to
> fallback to {{ShellBasedUnixGroupsMapping}}.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
