Prevent crashes for opencv exception and std::exception #14433

anirudh2290 · 2019-03-15T02:20:41Z

Description

fixes #8663 , fixes #9475, fixes #10389, fixes #13217

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

[] The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
Changes are complete (i.e. I finished coding on this PR)
All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)

Changes

Added support for std::exception handling to prevent crash
Added test

Comments

If this change is a backward incompatible change, why must this change be made.
Interesting edge cases to note here

karan6181 · 2019-03-15T22:45:23Z

@mxnet-label-bot add [Bug, OpenCV, Exception Handling, pr-awaiting-review]

yuxihu

Is this PR dependent on the waitall exception handling PR? I see duplicate code in these two PRs. Which one shall I review first?

anirudh2290 · 2019-03-20T01:48:34Z

This is dependent on waitall PR: #14397. Please review that one first.

wkcn

Thank you for the fix! LGTM

wkcn · 2019-03-31T15:53:04Z

src/resource.cc

-    gpu_parallel_rand_.Get(ctx.dev_id, [ctx, seed, this]() {
-      return new ResourceParallelRandom<gpu>(ctx, gpu_native_rand_copy_, seed);
-    })->Seed(seed);
+    if (ctx != Context::CPU()) {


ctx.dev_type == kGPU may be better.

piyushghai · 2019-04-09T00:48:21Z

@anirudh2290 Can you rebase with the master branch and resolve merge conflicts ?

anirudh2290 · 2019-04-09T21:27:22Z

@piyushghai done ! this is ready for review.

wkcn

LGTM. Thank you!

yuxihu

LGTM.

include/mxnet/c_api_error.h

anirudh2290 · 2019-04-19T18:25:17Z

@apeforest can you take a look too.

larroy

LGTM

larroy · 2019-04-22T18:33:53Z

include/mxnet/c_api_error.h

+#define MX_API_BEGIN()                                                         \
+  try {                                                                        \
+    on_enter_api(__FUNCTION__);
+#define MX_API_END()                                                           \


is it possible to have the defines inside the scopes a bit more ordered?

can you please elaborate on what you mean by that

Never mind, I think it's a separate discussion from this PR.

src/engine/threaded_engine.h

include/mxnet/c_api_error.h

anirudh2290 · 2019-04-22T23:26:13Z

@wkcn, @larroy and @yuxihu . Thanks a lot for your review. I have addressed/responded to your comments.

larroy · 2019-04-24T18:14:15Z

I approved already. Have a look at CI failures.

yuxihu

LGTM.

anirudhacharya · 2019-05-02T19:11:33Z

@mxnet-label-bot update [pr-awaiting-merge]

wkcn · 2019-05-05T04:13:00Z

Merged. Thank you!

* Relax constexpr restriction * Change imagenet_gen_qsym_mkldnn * Revert constexpr change * Add dummy image for test * Fix unreverted change * Remove url * Fix imagenet change * Catch only std::exception * Fix const, remove dmlc::Error catch * Retrigger CI

anirudh2290 marked this pull request as ready for review March 15, 2019 19:50

anirudh2290 requested review from nswamy and szha as code owners March 15, 2019 19:50

marcoabreu added Bug Exception Handling OpenCV OpenCV related issues pr-awaiting-review PR is waiting for code review labels Mar 15, 2019

yuxihu reviewed Mar 20, 2019

View reviewed changes

wkcn approved these changes Mar 31, 2019

View reviewed changes

anirudh2290 mentioned this pull request Apr 4, 2019

[Discussion] 1.5.0 Roadmap #14619

Closed

wkcn approved these changes Apr 9, 2019

View reviewed changes

yuxihu approved these changes Apr 12, 2019

View reviewed changes

larroy suggested changes Apr 12, 2019

View reviewed changes

include/mxnet/c_api_error.h Outdated Show resolved Hide resolved

anirudh2290 added 6 commits April 16, 2019 18:08

Relax constexpr restriction

dcd16ad

Change imagenet_gen_qsym_mkldnn

c0dcaf7

Revert constexpr change

aa6f3f2

Add dummy image for test

9cc0f14

Fix unreverted change

df5bc67

Remove url

798a5e0

anirudh2290 force-pushed the support_std_exception branch from 716715e to 798a5e0 Compare April 16, 2019 19:00

anirudh2290 added 2 commits April 16, 2019 19:01

Fix imagenet change

d96ca1c

Catch only std::exception

514c282

larroy approved these changes Apr 22, 2019

View reviewed changes

larroy reviewed Apr 22, 2019

View reviewed changes

yuxihu reviewed Apr 22, 2019

View reviewed changes

src/engine/threaded_engine.h Outdated Show resolved Hide resolved

include/mxnet/c_api_error.h Outdated Show resolved Hide resolved

include/mxnet/c_api_error.h Outdated Show resolved Hide resolved

Fix const, remove dmlc::Error catch

f2593d8

yuxihu approved these changes Apr 24, 2019

View reviewed changes

marcoabreu added pr-awaiting-merge Review and CI is complete. Ready to Merge and removed Bug Exception Handling OpenCV OpenCV related issues pr-awaiting-review PR is waiting for code review labels May 2, 2019

Retrigger CI

a42ec46

wkcn requested a review from eric-haibin-lin as a code owner May 5, 2019 01:55

wkcn merged commit 25ba1d1 into apache:master May 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent crashes for opencv exception and std::exception #14433

Prevent crashes for opencv exception and std::exception #14433

anirudh2290 commented Mar 15, 2019

karan6181 commented Mar 15, 2019

yuxihu left a comment

anirudh2290 commented Mar 20, 2019

wkcn left a comment •

edited

Loading

wkcn Mar 31, 2019

piyushghai commented Apr 9, 2019

anirudh2290 commented Apr 9, 2019

wkcn left a comment

yuxihu left a comment

anirudh2290 commented Apr 19, 2019

larroy left a comment

larroy Apr 22, 2019

anirudh2290 Apr 22, 2019

larroy Apr 24, 2019

anirudh2290 commented Apr 22, 2019 •

edited

Loading

larroy commented Apr 24, 2019 •

edited

Loading

yuxihu left a comment

anirudhacharya commented May 2, 2019

wkcn commented May 5, 2019

Prevent crashes for opencv exception and std::exception #14433

Prevent crashes for opencv exception and std::exception #14433

Conversation

anirudh2290 commented Mar 15, 2019

Description

Checklist

Essentials

Changes

Comments

karan6181 commented Mar 15, 2019

yuxihu left a comment

Choose a reason for hiding this comment

anirudh2290 commented Mar 20, 2019

wkcn left a comment • edited Loading

Choose a reason for hiding this comment

wkcn Mar 31, 2019

Choose a reason for hiding this comment

piyushghai commented Apr 9, 2019

anirudh2290 commented Apr 9, 2019

wkcn left a comment

Choose a reason for hiding this comment

yuxihu left a comment

Choose a reason for hiding this comment

anirudh2290 commented Apr 19, 2019

larroy left a comment

Choose a reason for hiding this comment

larroy Apr 22, 2019

Choose a reason for hiding this comment

anirudh2290 Apr 22, 2019

Choose a reason for hiding this comment

larroy Apr 24, 2019

Choose a reason for hiding this comment

anirudh2290 commented Apr 22, 2019 • edited Loading

larroy commented Apr 24, 2019 • edited Loading

yuxihu left a comment

Choose a reason for hiding this comment

anirudhacharya commented May 2, 2019

wkcn commented May 5, 2019

wkcn left a comment •

edited

Loading

anirudh2290 commented Apr 22, 2019 •

edited

Loading

larroy commented Apr 24, 2019 •

edited

Loading