You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by selvaganesang <gi...@git.apache.org> on 2016/12/02 18:50:19 UTC
[GitHub] incubator-trafodion pull request #861: [TRAFODION-1988] Better java exceptio...
GitHub user selvaganesang opened a pull request:
https://github.com/apache/incubator-trafodion/pull/861
[TRAFODION-1988] Better java exception handling in the java/JNI layer\u2026
\u2026 in TM
The following problems are fixed:
The loaded classes reference are made global to avoid the class reference from gc-ed
Introduced push/pop frame concepts to avoid memory growth usually observed with
many JNI/java transitions
Logged the errors via log4cxx mechanism instead of stdout
Possibly fixes the tm core dumped in obscure way at times
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/selvaganesang/incubator-trafodion trafodion-1988
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafodion/pull/861.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 #861
----
commit f502bc500a8ece5a07ff63dffd2a5d0410b2b8dc
Author: selvaganesang <se...@esgyn.com>
Date: 2016-12-02T18:35:45Z
[TRAFODION-1988] Better java exception handling in the java/JNI layer in TM
The following problems are fixed:
The loaded classes reference are made global to avoid the class reference from gc-ed
Introduced push/pop frame concepts to avoid memory growth usually observed with
many JNI/java transitions
Logged the errors via log4cxx mechanism instead of stdout
Possibly fixes the tm core dumped in obscure way at times
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #861: [TRAFODION-1988] Better java exceptio...
Posted by prashanth-vasudev <gi...@git.apache.org>.
Github user prashanth-vasudev commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/861#discussion_r91393909
--- Diff: core/sqf/src/seatrans/tm/hbasetmlib2/javaobjectinterfacetm.cpp ---
@@ -352,12 +349,11 @@ JOI_RetCode JavaObjectInterfaceTM::init(char* className,
// Allocate an object of the Java class, and call its constructor.
// The constructor must be the first entry in the methods array.
javaObj_ = _tlp_jenv->NewObject(javaClass, JavaMethods[0].methodID);
- if (javaObj_ == 0 || _tlp_jenv->ExceptionCheck())
- {
- _tlp_jenv->ExceptionDescribe();
- _tlp_jenv->ExceptionClear();
- _tlp_jenv->DeleteLocalRef(javaClass);
- return JOI_ERROR_NEWOBJ;
+ if (getExceptionDetails(NULL)) {
+ tm_log_write(DTM_TM_JNI_ERROR, SQ_LOG_ERR, (char *)"JavaObjectInterfaceTM::init()",
+ (char *)_tlp_error_msg->c_str(), -1LL);
+ _tlp_jenv->DeleteLocalRef(javaClass);
--- End diff --
Do you need to delete global reference javaclass here?
Also line 361.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #861: [TRAFODION-1988] Better java exceptio...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafodion/pull/861
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #861: [TRAFODION-1988] Better java exceptio...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/861#discussion_r91400045
--- Diff: core/sqf/src/seatrans/tm/hbasetmlib2/javaobjectinterfacetm.cpp ---
@@ -352,12 +349,11 @@ JOI_RetCode JavaObjectInterfaceTM::init(char* className,
// Allocate an object of the Java class, and call its constructor.
// The constructor must be the first entry in the methods array.
javaObj_ = _tlp_jenv->NewObject(javaClass, JavaMethods[0].methodID);
- if (javaObj_ == 0 || _tlp_jenv->ExceptionCheck())
- {
- _tlp_jenv->ExceptionDescribe();
- _tlp_jenv->ExceptionClear();
- _tlp_jenv->DeleteLocalRef(javaClass);
- return JOI_ERROR_NEWOBJ;
+ if (getExceptionDetails(NULL)) {
+ tm_log_write(DTM_TM_JNI_ERROR, SQ_LOG_ERR, (char *)"JavaObjectInterfaceTM::init()",
+ (char *)_tlp_error_msg->c_str(), -1LL);
+ _tlp_jenv->DeleteLocalRef(javaClass);
--- End diff --
In line 361 the global reference of the JavaClass needs to be retained. This ensures that the class reference is never unloaded via gc and all the method ids associated with the class remains valid. This was the one of the possible reasons for the obscure core seen in TM.
For the error path case, it should be ok to delete the global reference, but the TM process is unusable and it would be aborted anyway
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---