Skip to content

Commit

Permalink
[monodroid] Remove local reference to Java instance when done with it (
Browse files Browse the repository at this point in the history
…#1967)

Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/627319
Fixes: https://developercommunity.visualstudio.com/content/problem/268148/jni-local-reference-table-overflow-issue-in-xamari.html

Network interface information retrieval code needs to call a number
of Java methods in order to obtain the information from the OS.
A failure to free one of the retrieved local references caused a leak
and a possible eventual crash:

        JNI ERROR (app bug): local reference table overflow (max=512)
        local reference table dump:
        Last 10 entries (of 512):
        511: 0x12c53580 java.net.InterfaceAddress
        510: 0x12c1d640 byte[] (16 elements)
        509: 0x12c22358 java.net.Inet6Address
        508: 0x12c37ac0 java.net.InterfaceAddress[] (1 elements)
        507: 0x12c37ad0 java.net.InetAddress[] (1 elements)
        506: 0x12c53550 byte[] (6 elements)
        505: 0x12c1d780 java.lang.String "dummy0"
        504: 0x12c04610 java.net.NetworkInterface
        503: 0x12c37fe0 java.io.FileDescriptor
        502: 0x12c56228 com.android.server.NetworkManagementSocketTagger
        Summary:
        1 of java.net.Inet6Address
        2 of java.io.FileDescriptor (2 unique instances)
        2 of java.lang.String (2 unique instances)
        1 of java.net.InetAddress[] (1 elements)
        1 of byte[] (6 elements)
        1 of byte[] (16 elements)
        1 of java.net.InterfaceAddress[] (1 elements)
        500 of java.net.NetworkInterface (500 unique instances)
        1 of java.net.InterfaceAddress
        2 of com.android.server.NetworkManagementSocketTagger (1 unique instances)

Note the `500 of java.net.NetworkInterface` listing: that's the leak.

Call `JNIEnv::DeleteLocalRef()` on the `NetworkInterface` instance so
that we don't leak them, fixing the crash.

Lumped together is a potential NULL dereference fix and an
unnecessary signed comparison of unsigned operands.
  • Loading branch information
grendello authored and jonpryor committed Jul 17, 2018
1 parent 4a5dd01 commit 71a98b5
Showing 1 changed file with 17 additions and 4 deletions.
21 changes: 17 additions & 4 deletions src/monodroid/jni/monodroid-networkinfo.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ _monodroid_get_network_interface_state (const char *ifname, mono_bool *is_up, mo

pthread_once (&java_classes_once_control, java_classes_init);

JNIEnv *env = NULL;
jobject networkInterface = NULL;
if (!NetworkInterface_class || !NetworkInterface_getByName) {
if (!NetworkInterface_class)
log_warn (LOG_NET, "Failed to find the 'java.net.NetworkInterface' Java class");
Expand All @@ -77,10 +79,16 @@ _monodroid_get_network_interface_state (const char *ifname, mono_bool *is_up, mo
goto leave;
}

JNIEnv *env = get_jnienv ();
env = get_jnienv ();
jstring NetworkInterface_nameArg = (*env)->NewStringUTF (env, ifname);
jobject networkInterface = (*env)->CallStaticObjectMethod (env, NetworkInterface_class, NetworkInterface_getByName, NetworkInterface_nameArg);
networkInterface = (*env)->CallStaticObjectMethod (env, NetworkInterface_class, NetworkInterface_getByName, NetworkInterface_nameArg);
(*env)->DeleteLocalRef (env, NetworkInterface_nameArg);
if ((*env)->ExceptionOccurred (env)) {
log_warn (LOG_NET, "Java exception occurred while looking up the interface '%s'", ifname);
(*env)->ExceptionDescribe (env);
(*env)->ExceptionClear (env);
goto leave;
}

if (!networkInterface) {
log_warn (LOG_NET, "Failed to look up interface '%s' using Java API", ifname);
Expand All @@ -107,6 +115,11 @@ _monodroid_get_network_interface_state (const char *ifname, mono_bool *is_up, mo
leave:
if (!ret)
log_warn (LOG_NET, "Unable to determine interface '%s' state using Java API", ifname);

if (networkInterface != NULL && env != NULL) {
(*env)->DeleteLocalRef (env, networkInterface);
}

return ret;
}

Expand All @@ -128,11 +141,11 @@ _monodroid_get_network_interface_supports_multicast (const char *ifname, mono_bo
MONO_API int
_monodroid_get_dns_servers (void **dns_servers_array)
{
*dns_servers_array = NULL;
if (!dns_servers_array) {
log_warn (LOG_NET, "Unable to get DNS servers, no location to store data in");
return -1;
}
*dns_servers_array = NULL;

size_t len;
char *dns;
Expand All @@ -142,7 +155,7 @@ _monodroid_get_dns_servers (void **dns_servers_array)
for (int i = 0; i < 8; i++) {
prop_name [7] = (char)(i + 0x31);
len = monodroid_get_system_property (prop_name, &dns);
if (len <= 0) {
if (len == 0) {
dns_servers [i] = NULL;
continue;
}
Expand Down

1 comment on commit 71a98b5

@jahmai-ca
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably partly resolves https://bugzilla.xamarin.com/show_bug.cgi?id=52733
(finally...)

Please sign in to comment.