-
Notifications
You must be signed in to change notification settings - Fork 585
[GLUTEN-10797][CH] Reject all JNI calls after native destroy is invoked #10815
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Run Gluten Clickhouse CI on x86 |
fa1e5d7
to
2b164dd
Compare
Run Gluten Clickhouse CI on x86 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check the result status? maybe fail or something else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Useless, since active_executors_.clear();
will call dtor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are raw pointers, not shared_ptr
8d50c3e
to
6f234de
Compare
Run Gluten Clickhouse CI on x86 |
6f234de
to
feec026
Compare
Run Gluten Clickhouse CI on x86 |
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