IMPALA-8250: Clean up JNI warnings.

Using LIBHDFS_OPTS+="-Xcheck:jni" revealed a handful of warnings related to
(a) checking for exceptions and (b) leaking local references.

Checking for exceptions required sprinkling RETURN_ERROR_IF_EXC
left and right. I chose not to expand the JniCall infrastructure
to handle this more generally at the moment.

The leaky local references are a bit harder. In the logs, they show up
as "WARNING: JNI local refs: 2597, exceeds capacity: 35" or similar. A
few of these errors seem to be not in our code.  The ones that I've
found in our code stemmed from HBaseTableScanner::GetRowKey(): this
method uses local references and wasn't returning them. Using a
JniLocalFrame seems to have taken care of the warnings.

I have added code to skip test_large_strings when JNI checking is
enabled. This test takes forever (presumably because JNI is checking
bounds on strings very aggressively), and times out. The time out also
causes some metric-related checks to fail (since a query is still in
flight).

Debugging this required customizing my JDK to give stack traces
when these warnings occurred. The following diff facilitated
this.

  diff -r 76a9c9cf14f1 src/share/vm/prims/jniCheck.cpp
  --- a/src/share/vm/prims/jniCheck.cpp	Tue Jan 15 10:43:31 2019 +0000
  +++ b/src/share/vm/prims/jniCheck.cpp	Wed Feb 27 11:57:13 2019 -0800
  @@ -143,11 +143,30 @@
   static const char * fatal_instance_field_mismatch = "Field type (instance) mismatch in JNI get/set field operations";
   static const char * fatal_non_string = "JNI string operation received a non-string";

  +// thisone: whether to print every time, or maybe, depending on future
  +// how many future stacks we want printed (totally racy); helps catch
  +// missing exception handling if there's a way to tickle that code
  +// reliably.
  +static inline void dump_native_stack(JavaThread* thr, bool thisone, int future) {
  +  static int fut_stacks = 0; // racy!
  +  if (fut_stacks > 0) {
  +    thisone = true;
  +    fut_stacks--;
  +  }
  +  if (future > 0) fut_stacks = future;
  +  if (thisone) {
  +    frame fr = os::current_frame();
  +    char buf[6000];
  +    tty->print_cr("Thread: %s %d", thr->get_thread_name(), thr->osthread()->thread_id());
  +    print_native_stack(tty, fr, thr, buf, sizeof(buf));
  +  }
  +}

   // When in VM state:
   static void ReportJNIWarning(JavaThread* thr, const char *msg) {
     tty->print_cr("WARNING in native method: %s", msg);
     thr->print_stack();
  +  dump_native_stack(thr, true, 0);
   }

   // When in NATIVE state:
  @@ -199,11 +218,14 @@
         tty->print_cr("WARNING in native method: JNI call made without checking exceptions when required to from %s",
           thr->get_pending_jni_exception_check());
         thr->print_stack();
  +      dump_native_stack(thr, true, 10);
       )
       thr->clear_pending_jni_exception_check(); // Just complain once
     }
   }

  +
  +
   /**
    * Add to the planned number of handles. I.e. plus current live & warning threshold
    */
  @@ -254,9 +276,12 @@
         tty->print_cr("WARNING: JNI local refs: %zu, exceeds capacity: %zu",
             live_handles, planned_capacity);
         thr->print_stack();
  +      dump_native_stack(thr, true, 0);
       )
       // Complain just the once, reset to current + warn threshold
       add_planned_handle_capacity(handles, 0);
  +  } else {
  +    dump_native_stack(thr, false, 0);
     }
   }

Change-Id: Idd1709f749a764c1d947704bc64306493863b45f
Reviewed-on: http://gerrit.cloudera.org:8080/12660
Reviewed-by: Joe McDonnell <joemcdonnell@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>

tests/query_test/test_insert.py

@@ -17,6 +17,7 @@
 
 # Targeted Impala insert tests
 
+import os
 import pytest
 
 from testdata.common import widetable
@@ -81,7 +82,8 @@ class TestInsertQueries(ImpalaTestSuite):
   @pytest.mark.execute_serially
   def test_insert_large_string(self, vector, unique_database):
     """Test handling of large strings in inserter and scanner."""
-    table_format = vector.get_value('table_format')
+    if "-Xcheck:jni" in os.environ.get("LIBHDFS_OPTS", ""):
+      pytest.skip("Test unreasonably slow with JNI checking.")
     table_name = unique_database + ".insert_largestring"
 
     file_format = vector.get_value('table_format').file_format