[GLUTEN-10797][CH] Reject all JNI calls after native destroy is invoked#10815
[GLUTEN-10797][CH] Reject all JNI calls after native destroy is invoked#10815lgbo-ustc merged 1 commit intoapache:mainfrom
Conversation
|
Run Gluten Clickhouse CI on x86 |
lgbo-ustc
force-pushed
the
bug_0923_core
branch
from
fa1e5d7 to
2b164dd
Compare
|
Run Gluten Clickhouse CI on x86 |
zhztheplayer
changed the title
lgbo-ustc
force-pushed
the
bug_0923_core
branch
from
2b164dd to
8d50c3e
Compare
|
Run Gluten Clickhouse CI on x86 |
| for (auto it = active_executors_.begin(); it != active_executors_.end(); ++it) | ||
| { | ||
| auto * executor = *it; | ||
| executor->cancel(); |
There was a problem hiding this comment.
Should we check the result status? maybe fail or something else.
There was a problem hiding this comment.
the result type of cancel is void
| auto * executor = *it; | ||
| executor->cancel(); | ||
| std::cerr << "Not closed LocalExecutor:\n" << executor->dumpPipeline() << std::endl; | ||
| delete executor; |
There was a problem hiding this comment.
Useless, since active_executors_.clear(); will call dtor.
There was a problem hiding this comment.
these are raw pointers, not shared_ptr
lgbo-ustc
force-pushed
the
bug_0923_core
branch
from
8d50c3e to
6f234de
Compare
|
Run Gluten Clickhouse CI on x86 |
lgbo-ustc
force-pushed
the
bug_0923_core
branch
from
6f234de to
feec026
Compare
|
Run Gluten Clickhouse CI on x86 |
lgbo-ustc
force-pushed
the
bug_0923_core
branch
from
feec026 to
9d5e262
Compare
|
Run Gluten Clickhouse CI on x86 |
lgbo-ustc
force-pushed
the
bug_0923_core
branch
from
9d5e262 to
b6e527d
Compare
|
Run Gluten Clickhouse CI on x86 |
lgbo-ustc
force-pushed
the
bug_0923_core
branch
from
b6e527d to
c1a0849
Compare
|
Run Gluten Clickhouse CI on x86 |
lgbo-ustc
force-pushed
the
bug_0923_core
branch
from
c1a0849 to
172135a
Compare
|
Run Gluten Clickhouse CI on x86 |
lgbo-ustc
force-pushed
the
bug_0923_core
branch
from
172135a to
0fedc0f
Compare
|
Run Gluten Clickhouse CI on x86 |
| jbyteArray stringTojbyteArray(JNIEnv * env, const std::string & str); | ||
|
|
||
| // A watcher to monitor JNIEnv status: it's active or not | ||
| class JniEnvStatusWatcher |
There was a problem hiding this comment.
Better add
JniEnvStatusWatcher() = default;
~JniEnvStatusWatcher() = default;
and set to private to avoid compiler autogenerate. Same as above other singleton classes.
cpp-ch/local-engine/jni/jni_common.h
Outdated
| { | ||
| public: | ||
| static JniEnvStatusWatcher & instance(); | ||
| void active() { is_active.store(true, std::memory_order_release); } |
There was a problem hiding this comment.
Better to use setActive name as it does.
cpp-ch/local-engine/jni/jni_common.h
Outdated
| public: | ||
| static JniEnvStatusWatcher & instance(); | ||
| void active() { is_active.store(true, std::memory_order_release); } | ||
| void inactive() { is_active.store(false, std::memory_order_release); } |
There was a problem hiding this comment.
Better use setInactive name as it does.
lgbo-ustc
force-pushed
the
bug_0923_core
branch
from
0fedc0f to
cdbd864
Compare
|
Run Gluten Clickhouse CI on x86 |
| */ | ||
| #include "QueryContext.h" | ||
|
|
||
| #include <cstdint> |
There was a problem hiding this comment.
introduced by autocomplete
lgbo-ustc
force-pushed
the
bug_0923_core
branch
from
cdbd864 to
d0b1e36
Compare
|
Run Gluten Clickhouse CI on x86 |
|
Good job and hope we can solve the early stop causing crash on CH BE. |
3 participants
What changes are proposed in this pull request?
Reject all JNI calls after native destroy is invoked, to prevent access to invalid resources.
How was this patch tested?
test manually