anirudh2290 commented on a change in pull request #9681: Better Exception 
Handling for Operators
URL: https://github.com/apache/incubator-mxnet/pull/9681#discussion_r165781254
 
 

 ##########
 File path: src/engine/threaded_engine.h
 ##########
 @@ -338,33 +346,46 @@ class ThreadedEngine : public Engine {
 #endif
     CallbackOnComplete callback = this->CreateCallback(
         ThreadedEngine::OnCompleteStatic, opr_block);
+    CallbackOnComplete on_start_callback = this->CreateCallback(
+        ThreadedEngine::OnStartStatic, opr_block);
     bool debug_info = (engine_info_ && debug_push_opr_ == opr_block);
     if (debug_info) {
       LOG(INFO) << "ExecuteOprBlock " << opr_block
                 << "shutdown_phase=" << shutdown_phase_;
     }
     if (!shutdown_phase_) {
       try {
+        on_start_callback();
         if (debug_info) {
           LOG(INFO) << "ExecuteOprFn ";
         }
-        threaded_opr->fn(run_ctx, callback);
+        try {
+          if (!threaded_opr->ex_ptr || threaded_opr->wait) {
+            threaded_opr->fn(run_ctx, callback);
+          } else {
+            callback();
+          }
+        } catch (dmlc::Error& e) {
+          threaded_opr->ex_ptr = std::current_exception();
+          callback();
+        }
         if (debug_info) {
           LOG(INFO) << "Fin ExecuteOprFn ";
         }
-      } catch(dmlc::Error &e) {
+      } catch (dmlc::Error& e) {
 
 Review comment:
   @piiswrong my intention for the outer block was to catch all other 
exceptions which are not caught in the inner block. I should change dmlc::Error 
to std::exception to catch all standard exception. But you make a good point 
about propagating all the exceptions and not just dmlc::Error to the frontend. 
We can take two approaches here: 1. catch dmlc::Error and terminate the process 
for everything else. 2. catch both dmlc::Error and standard exceptions and 
propagate to frontend. This will require code changes to the guards , 
c_api_error and potentially frontend code. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to