Merge pull request dealii#18921 from gassmoeller/increase_safety_of_timer

Author: masterleinad

include/deal.II/base/timer.h

@@ -203,6 +203,12 @@ class Timer
   void
   restart();
 
+  /**
+   * Returns true if the timer is currently running, false otherwise.
+   */
+  bool
+  is_running() const;
+
   /**
    * Return the current accumulated wall time (including the current lap, if
    * the timer is running) in seconds without stopping the timer.
@@ -384,6 +390,17 @@ class Timer
    * the current lap is included in the count.
    */
   unsigned int n_timed_laps;
+
+  /**
+   * A lock that makes sure that this class gives reasonable results even when
+   * used with several threads. Note that thread-safety with a timer that is
+   * shared between different threads is hard to establish, and it is in
+   * particular discouraged to have multiple threads starting and stopping
+   * the timer. This case only works if each thread makes sure the timer
+   * is not already running before starting it, and stopping the timer
+   * before any other thread can use it.
+   */
+  Threads::Mutex mutex;
 };
 
 

source/base/timer.cc

@@ -175,7 +175,10 @@ Timer::Timer(const MPI_Comm mpi_communicator, const bool sync_lap_times_)
 void
 Timer::start()
 {
-  if (running == false)
+  // Make sure only one thread at a time starts or resets the timer
+  std::lock_guard<std::mutex> lock(mutex);
+
+  if (is_running() == false)
     ++n_timed_laps;
 
   running = true;
@@ -195,47 +198,57 @@ Timer::start()
 void
 Timer::stop()
 {
-  if (running)
-    {
-      running = false;
+  // Make sure only one thread at a time stops the timer
+  std::lock_guard<std::mutex> lock(mutex);
 
-      wall_times.last_lap_time =
-        wall_clock_type::now() - wall_times.current_lap_start_time;
-      cpu_times.last_lap_time =
-        cpu_clock_type::now() - cpu_times.current_lap_start_time;
+  AssertThrow(is_running() == true,
+              ExcMessage(
+                "This timer is not running and hence cannot be stopped."));
 
-      last_lap_wall_time_data =
-        Utilities::MPI::min_max_avg(internal::TimerImplementation::to_seconds(
-                                      wall_times.last_lap_time),
-                                    mpi_communicator);
-      if (sync_lap_times)
-        {
-          wall_times.last_lap_time =
-            internal::TimerImplementation::from_seconds<
-              decltype(wall_times)::duration_type>(last_lap_wall_time_data.max);
-          cpu_times.last_lap_time = internal::TimerImplementation::from_seconds<
-            decltype(cpu_times)::duration_type>(
-            Utilities::MPI::min_max_avg(
-              internal::TimerImplementation::to_seconds(
-                cpu_times.last_lap_time),
-              mpi_communicator)
-              .max);
-        }
-      wall_times.accumulated_time += wall_times.last_lap_time;
-      cpu_times.accumulated_time += cpu_times.last_lap_time;
-      accumulated_wall_time_data =
+  running = false;
+
+  wall_times.last_lap_time =
+    wall_clock_type::now() - wall_times.current_lap_start_time;
+  cpu_times.last_lap_time =
+    cpu_clock_type::now() - cpu_times.current_lap_start_time;
+
+  last_lap_wall_time_data =
+    Utilities::MPI::min_max_avg(internal::TimerImplementation::to_seconds(
+                                  wall_times.last_lap_time),
+                                mpi_communicator);
+  if (sync_lap_times)
+    {
+      wall_times.last_lap_time = internal::TimerImplementation::from_seconds<
+        decltype(wall_times)::duration_type>(last_lap_wall_time_data.max);
+      cpu_times.last_lap_time = internal::TimerImplementation::from_seconds<
+        decltype(cpu_times)::duration_type>(
         Utilities::MPI::min_max_avg(internal::TimerImplementation::to_seconds(
-                                      wall_times.accumulated_time),
-                                    mpi_communicator);
+                                      cpu_times.last_lap_time),
+                                    mpi_communicator)
+          .max);
     }
+  wall_times.accumulated_time += wall_times.last_lap_time;
+  cpu_times.accumulated_time += cpu_times.last_lap_time;
+  accumulated_wall_time_data =
+    Utilities::MPI::min_max_avg(internal::TimerImplementation::to_seconds(
+                                  wall_times.accumulated_time),
+                                mpi_communicator);
+}
+
+
+
+bool
+Timer::is_running() const
+{
+  return running;
 }
 
 
 
 double
 Timer::cpu_time() const
 {
-  if (running)
+  if (is_running())
     {
       const double running_time = internal::TimerImplementation::to_seconds(
         cpu_clock_type::now() - cpu_times.current_lap_start_time +
@@ -264,7 +277,7 @@ double
 Timer::wall_time() const
 {
   wall_clock_type::duration current_elapsed_wall_time;
-  if (running)
+  if (is_running())
     current_elapsed_wall_time = wall_clock_type::now() -
                                 wall_times.current_lap_start_time +
                                 wall_times.accumulated_time;
@@ -435,7 +448,13 @@ TimerOutput::enter_subsection(const std::string &section_name)
         sections[section_name] = Timer(mpi_communicator, false);
     }
   else
-    sections[section_name].start();
+    {
+      Assert(sections[section_name].is_running() == false,
+             ExcMessage("Cannot enter the already active section <" +
+                        section_name + ">."));
+
+      sections[section_name].start();
+    }
 
   active_sections.push_back(section_name);
 }
@@ -465,6 +484,10 @@ TimerOutput::leave_subsection(const std::string &section_name)
   const std::string actual_section_name =
     (section_name.empty() ? active_sections.back() : section_name);
 
+  Assert(sections[actual_section_name].is_running() == true,
+         ExcMessage("Cannot leave section <" + actual_section_name +
+                    ">, because it has not been entered."));
+
   sections[actual_section_name].stop();
 
   // in case we have to print out something, do that here...

tests/base/timer_11.cc

@@ -0,0 +1,33 @@
+// ------------------------------------------------------------------------
+//
+// SPDX-License-Identifier: LGPL-2.1-or-later
+// Copyright (C) 2017 - 2018 by the deal.II authors
+//
+// This file is part of the deal.II library.
+//
+// Part of the source code is dual licensed under Apache-2.0 WITH
+// LLVM-exception OR LGPL-2.1-or-later. Detailed license information
+// governing the source code and code contributions can be found in
+// LICENSE.md and CONTRIBUTING.md at the top level directory of deal.II.
+//
+// ------------------------------------------------------------------------
+
+// test that a timer cannot be stopped while not running
+
+#include <deal.II/base/timer.h>
+
+#include "../tests.h"
+
+int
+main()
+{
+  initlog();
+
+  Timer t;
+  t.stop();
+
+  // this will throw an exception
+  t.stop();
+
+  deallog << "OK" << std::endl;
+}

tests/base/timer_11.expect=run.output

@@ -0,0 +1,3 @@
+
+DEAL::we should not get here
+DEAL::