Redress review comments.

include/caffe/solver.hpp

@@ -8,10 +8,27 @@
 
 namespace caffe {
 
+/**
+  * @brief Enumeration of actions that a client of the Solver may request by
+  * implementing the Solver's action request function, which a
+  * a client may optionally provide in order to request early termination
+  * or saving a snapshot without exiting. In the executable caffe, this
+  * mechanism is used to allow the snapshot to be saved when stopping
+  * execution with a SIGINT (Ctrl-C).
+  */
+  namespace SolverAction {
+    enum Enum {
+      NONE = 0, // Take no special action.
+      STOP = 1, // Stop training. snapshot_after_train controls whether a
+                // snapshot is created.
+      SNAPSHOT = 2 // Take a snapshot, and keep training.
+    };
+  }
+
 /**
  * @brief Type of a function that returns a Solver Action enumeration.
  */
-typedef boost::function<SolverParameter_Action()> ActionCallback;
+typedef boost::function<SolverAction::Enum()> ActionCallback;
 
 /**
  * @brief An interface for classes that perform optimization on Net%s.
@@ -33,7 +50,7 @@ class Solver {
   // that the solver uses to see what action it should take (e.g. snapshot or
   // exit training early).
   void SetActionFunction(ActionCallback func);
-  SolverParameter_Action GetRequestedAction();
+  SolverAction::Enum GetRequestedAction();
   // The main entry of the solver function. In default, iter will be zero. Pass
   // in a non-zero iter number to resume training for a pre-trained net.
   virtual void Solve(const char* resume_file = NULL);

include/caffe/util/signal_handler.h

@@ -9,14 +9,14 @@ namespace caffe {
 class SignalHandler {
  public:
   // Contructor. Specify what action to take when a signal is received.
-  SignalHandler(SolverParameter_Action SIGINT_action,
-                SolverParameter_Action SIGHUP_action);
+  SignalHandler(SolverAction::Enum SIGINT_action,
+                SolverAction::Enum SIGHUP_action);
+  ~SignalHandler();
   ActionCallback GetActionFunction();
  private:
-  SignalHandler();  // Not implemented.
-  SolverParameter_Action CheckForSignals() const;
-  SolverParameter_Action SIGINT_action_;
-  SolverParameter_Action SIGHUP_action_;
+  SolverAction::Enum CheckForSignals() const;
+  SolverAction::Enum SIGINT_action_;
+  SolverAction::Enum SIGHUP_action_;
 };
 
 }  // namespace caffe

src/caffe/proto/caffe.proto

@@ -234,19 +234,6 @@ message SolverParameter {
 
   // If false, don't save a snapshot after training finishes.
   optional bool snapshot_after_train = 28 [default = true];
-
-  // Enumeration of actions that a client of the Solver may request by
-  // implementing the Solver's action request function, which a
-  // a client may optionally provide in order to request early termination
-  // or saving a snapshot without exiting. In the executable caffe, this
-  // mechanism is used to allow the snapshot to be saved when stopping
-  // execution with a SIGINT (Ctrl-C).
-  enum Action {
-    NONE = 0; // Take no special action.
-    STOP = 1; // Stop training. snapshot_after_train controls whether a snapshot
-              // is created.
-    SNAPSHOT = 2; // Take a snapshot, and keep training.
-  }
 }
 
 // A message that stores the solver snapshots

src/caffe/solver.cpp

@@ -23,12 +23,12 @@ void Solver<Dtype>::SetActionFunction(ActionCallback func) {
 }
 
 template<typename Dtype>
-SolverParameter_Action Solver<Dtype>::GetRequestedAction() {
+SolverAction::Enum Solver<Dtype>::GetRequestedAction() {
   if (action_request_function_) {
     // If the external request function has been set, call it.
     return action_request_function_();
   }
-  return SolverParameter_Action_NONE;
+  return SolverAction::NONE;
 }
 
 template <typename Dtype>
@@ -270,16 +270,16 @@ void Solver<Dtype>::Step(int iters) {
     // the number of times the weights have been updated.
     ++iter_;
 
-    SolverParameter_Action request = GetRequestedAction();
+    SolverAction::Enum request = GetRequestedAction();
 
     // Save a snapshot if needed.
     if ((param_.snapshot()
          && iter_ % param_.snapshot() == 0
          && Caffe::root_solver()) ||
-         (request == SolverParameter_Action_SNAPSHOT)) {
+         (request == SolverAction::SNAPSHOT)) {
       Snapshot();
     }
-    if (SolverParameter_Action_STOP == request) {
+    if (SolverAction::STOP == request) {
       requested_early_exit_ = true;
       // Break out of training loop.
       break;
@@ -353,12 +353,12 @@ void Solver<Dtype>::Test(const int test_net_id) {
   const shared_ptr<Net<Dtype> >& test_net = test_nets_[test_net_id];
   Dtype loss = 0;
   for (int i = 0; i < param_.test_iter(test_net_id); ++i) {
-    SolverParameter_Action request = GetRequestedAction();
+    SolverAction::Enum request = GetRequestedAction();
     // Check to see if stoppage of testing/training has been requested.
-    while (request != SolverParameter_Action_NONE) {
-        if (SolverParameter_Action_SNAPSHOT == request) {
+    while (request != SolverAction::NONE) {
+        if (SolverAction::SNAPSHOT == request) {
           Snapshot();
-        } else if (SolverParameter_Action_STOP == request) {
+        } else if (SolverAction::STOP == request) {
           requested_early_exit_ = true;
         }
         request = GetRequestedAction();

src/caffe/util/signal_handler.cpp

@@ -29,7 +29,7 @@ namespace {
     already_hooked_up = true;
 
     struct sigaction sa;
-    // Setup the sighub handler
+    // Setup the handler
     sa.sa_handler = &handle_signal;
     // Restart the system call, if at all possible
     sa.sa_flags = SA_RESTART;
@@ -44,6 +44,28 @@ namespace {
     }
   }
 
+  // Set the signal handlers to the default.
+  void UnhookHandler() {
+    if (already_hooked_up) {
+      struct sigaction sa;
+      // Setup the sighub handler
+      sa.sa_handler = SIG_DFL;
+      // Restart the system call, if at all possible
+      sa.sa_flags = SA_RESTART;
+      // Block every signal during the handler
+      sigfillset(&sa.sa_mask);
+      // Intercept SIGHUP and SIGINT
+      if (sigaction(SIGHUP, &sa, NULL) == -1) {
+        LOG(FATAL) << "Cannot install SIGHUP handler.";
+      }
+      if (sigaction(SIGINT, &sa, NULL) == -1) {
+        LOG(FATAL) << "Cannot install SIGINT handler.";
+      }
+
+      already_hooked_up = false;
+    }
+  }
+
   // Return true iff a SIGINT has been received since the last time this
   // function was called.
   bool GotSIGINT() {
@@ -63,21 +85,25 @@ namespace {
 
 namespace caffe {
 
-SignalHandler::SignalHandler(SolverParameter_Action SIGINT_action,
-                             SolverParameter_Action SIGHUP_action):
+SignalHandler::SignalHandler(SolverAction::Enum SIGINT_action,
+                             SolverAction::Enum SIGHUP_action):
   SIGINT_action_(SIGINT_action),
   SIGHUP_action_(SIGHUP_action) {
   HookupHandler();
 }
 
-SolverParameter_Action SignalHandler::CheckForSignals() const {
+SignalHandler::~SignalHandler() {
+  UnhookHandler();
+}
+
+SolverAction::Enum SignalHandler::CheckForSignals() const {
   if (GotSIGHUP()) {
     return SIGHUP_action_;
   }
   if (GotSIGINT()) {
     return SIGINT_action_;
   }
-  return SolverParameter_Action_NONE;
+  return SolverAction::NONE;
 }
 
 // Return the function that the solver can use to find out if a snapshot or

tools/caffe.cpp

@@ -135,16 +135,16 @@ void CopyLayers(caffe::Solver<float>* solver, const std::string& model_list) {
 
 // Translate the signal effect the user specified on the command-line to the
 // corresponding enumeration.
-caffe::SolverParameter_Action GetRequestedAction(
+caffe::SolverAction::Enum GetRequestedAction(
     const std::string& flag_value) {
   if (flag_value == "stop") {
-    return caffe::SolverParameter_Action_STOP;
+    return caffe::SolverAction::STOP;
   }
   if (flag_value == "snapshot") {
-    return caffe::SolverParameter_Action_SNAPSHOT;
+    return caffe::SolverAction::SNAPSHOT;
   }
   if (flag_value == "none") {
-    return caffe::SolverParameter_Action_NONE;
+    return caffe::SolverAction::NONE;
   }
   LOG(FATAL) << "Invalid signal effect \""<< flag_value << "\" was specified";
 }