Introduce AutoHCK::ResourceScope

Before this change, the management of resources that require explicit deallocations were done ad-hoc and inconsistent. This new class provides a unified mechanism for this purpose and ensures the following: - Resource deallocation happens in reverse-allocation order. This is important because an object often depend on another object allocated earlier and cannot outlive the earlier object. - Interrupts during deallocation will be postponed. (Ensured with Thread.handle_interrupt(Object => :never)) - An interrupt won't be handled while an allocated object is assigned to a AutoHCK::ResourceScope. (Ensured with Thread.handle_interrupt(Object => :on_blocking)) Moreover, this class facilitates error handling during resource allocation. A resource allocation procedure can be wrapped with AutoHCK::ResourceScope.open, and end with AutoHCK::ResourceScope#move_to call with the outer scope as an argument; if an error happens during the allocation, resources already allocated will be safely deallocated. If no error occurs, the allocated resources will escape the inner scope with AutoHCK::ResourceScope#move_to call. Signed-off-by: Akihiko Odaki <[email protected]>
HCK-CI · Jul 13, 2023 · 1b276f3 · 1b276f3
1 parent f55b65d
commit 1b276f3
Show file tree

Hide file tree

Showing 17 changed files with 334 additions and 372 deletions.
diff --git a/bin/auto_hck b/bin/auto_hck
@@ -7,6 +7,7 @@ require './lib/config/sentry'
 
 begin
   require 'filelock'
+  require './lib/auxiliary/resource_scope'
   require './lib/cli'
   require './lib/project'
   require './lib/trap'
@@ -25,8 +26,8 @@ begin
     Thread.abort_on_exception = true
     Thread.report_on_exception = false
 
-    @project = Project.new(cli)
-    begin
+    ResourceScope.open do |scope|
+      @project = Project.new(scope, cli)
       Trap.project = @project
       @project.run if @project.prepare
     rescue StandardError => e
@@ -37,9 +38,9 @@ begin
       @project.log_exception(e, 'fatal')
       @project.handle_error
       exit(1)
-    ensure
-      @project.close
     end
+  rescue AutoHCKInterrupt
+    exit false
   end
 rescue StandardError => e
   Sentry.capture_exception(e)

diff --git a/lib/auxiliary/cmd_run.rb b/lib/auxiliary/cmd_run.rb
@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'tempfile'
+require_relative 'resource_scope'
 
 # AutoHCK module
 module AutoHCK
@@ -42,22 +43,18 @@ def wait_for_status(flags)
       _, status = Process.wait2(@pid, flags)
       return if status.nil?
 
-      begin
-        begin
-          @stdout.rewind
-          stdout = @stdout.read
-        ensure
-          @stdout.close
-        end
+      ResourceScope.open do |scope|
+        scope << @stdout
+        scope << @stderr
 
+        @stdout.rewind
+        stdout = @stdout.read
         @stderr.rewind
         stderr = @stderr.read
-      ensure
-        @stderr.close
-      end
 
-      @logger.info("Info dump:#{prep_log_stream(stdout)}") unless stdout.empty?
-      @logger.warn("Error dump:#{prep_log_stream(stderr)}") unless stderr.empty?
+        @logger.info("Info dump:#{prep_log_stream(stdout)}") unless stdout.empty?
+        @logger.warn("Error dump:#{prep_log_stream(stderr)}") unless stderr.empty?
+      end
 
       yield status
 

diff --git a/lib/auxiliary/id_gen.rb b/lib/auxiliary/id_gen.rb
@@ -7,11 +7,12 @@
 module AutoHCK
   # Id Generator class
   class Idgen
-    def initialize(range, timeout)
+    def initialize(scope, range, timeout)
       @db = './id_gen.db'
       @range = range
-      @conn = SQLite3::Database.new @db.to_s
       @threshold = timeout * 24 * 60 * 60
+      @conn = SQLite3::Database.new @db.to_s
+      scope << @conn
     end
 
     def load_data
@@ -55,8 +56,6 @@ def release(id)
       1
     rescue SQLite3::Exception
       -1
-    ensure
-      @conn&.close
     end
 
     private

diff --git a/lib/auxiliary/resource_scope.rb b/lib/auxiliary/resource_scope.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+require './lib/exceptions'
+
+module AutoHCK
+  # ResourceScope is a class that manages objects with close methods.
+  class ResourceScope
+    def initialize
+      @resources = []
+    end
+
+    def <<(resource)
+      @resources << resource
+    end
+
+    def close
+      @resources.reverse_each(&:close)
+      @resources.clear
+    end
+
+    def concat(...)
+      @resources.concat(...)
+    end
+
+    def move_to(another)
+      another.concat @resources
+      @resources.clear
+    end
+
+    def self.open
+      scope = new
+      Thread.handle_interrupt(AutoHCKInterrupt => :never) do
+        Thread.handle_interrupt(AutoHCKInterrupt => :on_blocking) do
+          yield scope
+        end
+      ensure
+        scope.close
+      end
+    end
+  end
+end
diff --git a/lib/engines/config_manager/config_manager.rb b/lib/engines/config_manager/config_manager.rb
@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'fileutils'
+require './lib/auxiliary/resource_scope'
 require './lib/resultuploaders/result_uploader'
 
 # AutoHCK module
@@ -34,9 +35,11 @@ def tag
     end
 
     def run
-      @project.logger.info('Stating result uploader token initialization')
-      @result_uploader = ResultUploader.new(@project)
-      @result_uploader.ask_token
+      ResourceScope.open do |scope|
+        @project.logger.info('Stating result uploader token initialization')
+        @result_uploader = ResultUploader.new(scope, @project)
+        @result_uploader.ask_token
+      end
     end
 
     def drivers
@@ -50,9 +53,5 @@ def platform
     def result_uploader_needed?
       false
     end
-
-    def close
-      @project.logger.info('Closing helpers engine')
-    end
   end
 end
diff --git a/lib/engines/hckinstall/hckinstall.rb b/lib/engines/hckinstall/hckinstall.rb
@@ -7,6 +7,7 @@
 require './lib/auxiliary/json_helper'
 require './lib/auxiliary/host_helper'
 require './lib/auxiliary/iso_helper'
+require './lib/auxiliary/resource_scope'
 require './lib/engines/hckinstall/setup_scripts_helper'
 
 # AutoHCK module
@@ -188,17 +189,17 @@ def result_uploader_needed?
       true
     end
 
-    def run_studio(iso_list = [], keep_alive:, snapshot: true)
+    def run_studio(scope, iso_list = [], keep_alive:, snapshot: true)
       st_opts = {
         keep_alive: keep_alive,
         create_snapshot: snapshot,
         attach_iso_list: iso_list
       }
 
-      @project.setup_manager.run_hck_studio(st_opts)
+      @project.setup_manager.run_hck_studio(scope, st_opts)
     end
 
-    def run_client(name, snapshot: true)
+    def run_client(scope, studio, name, snapshot: true)
       cl_opts = {
         create_snapshot: snapshot,
         attach_iso_list: [
@@ -207,48 +208,40 @@ def run_client(name, snapshot: true)
         ]
       }
 
-      @project.setup_manager.run_hck_client(name, cl_opts)
+      @project.setup_manager.run_hck_client(scope, studio, name, cl_opts)
     end
 
     def run_studio_installer
       @project.setup_manager.create_studio_image
 
-      st = run_studio([
-                        @setup_studio_iso,
-                        @studio_iso_info['path']
-                      ], keep_alive: false, snapshot: false)
-      begin
+      ResourceScope.open do |scope|
+        st = run_studio(scope, [
+                          @setup_studio_iso,
+                          @studio_iso_info['path']
+                        ], keep_alive: false, snapshot: false)
         Timeout.timeout(@studio_install_timeout) do
           @logger.info('Waiting for studio installation finished')
           sleep 5 while st.alive?
         end
-      ensure
-        st.clean_last_run
       end
     end
 
-    def run_client_installer(name)
+    def run_client_installer(scope, studio, name)
       @project.setup_manager.create_client_image(name)
 
-      run_client(name, snapshot: false)
+      run_client(scope, studio, name, snapshot: false)
     end
 
     def run_clients_installer
-      st = run_studio([], keep_alive: true)
-      begin
-        cl = @clients_name.map { |c| run_client_installer(c) }
-        begin
-          Timeout.timeout(@client_install_timeout) do
-            cl.each do |client|
-              @logger.info("Waiting for #{client.name} installation finished")
-              sleep 5 while client.alive?
-            end
+      ResourceScope.open do |scope|
+        st = run_studio(scope, [], keep_alive: true)
+        cl = @clients_name.map { |c| run_client_installer(scope, st, c) }
+        Timeout.timeout(@client_install_timeout) do
+          cl.each do |client|
+            @logger.info("Waiting for #{client.name} installation finished")
+            sleep 5 while client.alive?
           end
-        ensure
-          cl.each(&:clean_last_run)
         end
-      ensure
-        st.clean_last_run
       end
     end
 

diff --git a/lib/engines/hcktest/hcktest.rb b/lib/engines/hcktest/hcktest.rb
@@ -4,6 +4,7 @@
 require './lib/setupmanagers/hckclient'
 require './lib/auxiliary/diff_checker'
 require './lib/auxiliary/json_helper'
+require './lib/auxiliary/resource_scope'
 require './lib/auxiliary/zip_helper'
 
 # AutoHCK module
@@ -144,14 +145,14 @@ def read_platform
       Json.read_json(platform_json, @logger)
     end
 
-    def run_studio(run_opts = {})
-      @studio = @project.setup_manager.run_hck_studio(run_opts)
+    def run_studio(scope, run_opts = {})
+      @studio = @project.setup_manager.run_hck_studio(scope, run_opts)
     end
 
-    def run_clients(run_opts = {})
+    def run_clients(scope, run_opts = {})
       @clients = {}
       @platform['clients'].each do |_name, client|
-        @clients[client['name']] = @project.setup_manager.run_hck_client(client['name'], run_opts)
+        @clients[client['name']] = @project.setup_manager.run_hck_client(scope, @studio, client['name'], run_opts)
 
         break if @project.options.test.svvp
         break unless @drivers.any? { |d| d['support'] }
@@ -162,12 +163,6 @@ def run_clients(run_opts = {})
                                 this platform is incorrect'
     end
 
-    def synchronize_clients(exit: false)
-      @clients.each_value do |client|
-        client.synchronize(exit: exit)
-      end
-    end
-
     def configure_clients
       run_only = @project.options.test.manual && @project.options.test.driver_path.nil?
 
@@ -179,35 +174,29 @@ def configure_clients
     def configure_setup_and_synchronize
       @studio.configure(@platform['clients'])
       configure_clients
-      synchronize_clients
+      @clients.each_value(&:synchronize)
       @client1 = @clients.values[0]
       @client2 = @clients.values[1]
       @client1.support = @client2
+      @studio.keep_snapshot
+      @clients.each_value(&:keep_snapshot)
     end
 
-    def clean_last_run_clients
-      @clients.values.map(&:clean_last_run)
-    end
-
-    def clean_last_run_machines
-      @studio.clean_last_run
-      clean_last_run_clients
-    end
-
-    def run_and_configure_setup
+    def run_and_configure_setup(scope)
       retries ||= 0
 
-      run_studio
-      sleep 5 until @studio.up?
-      run_clients keep_alive: true
+      ResourceScope.open do |tmp_scope|
+        run_studio tmp_scope
+        sleep 5 until @studio.up?
+        run_clients tmp_scope, keep_alive: true
 
-      configure_setup_and_synchronize
+        configure_setup_and_synchronize
+        tmp_scope.move_to scope
+      end
     rescue AutoHCKError => e
-      synchronize_clients(exit: true)
       @project.logger.warn("Running and configuring setup failed: (#{e.class}) #{e.message}")
       raise e unless (retries += 1) < AUTOHCK_RETRIES
 
-      clean_last_run_machines
       @project.logger.info('Trying again to run and configure setup')
       retry
     end
@@ -230,19 +219,18 @@ def tag
     end
 
     def manual_run
-      run_studio({ dump_only: true })
-      run_clients({ dump_only: true })
+      ResourceScope.open do |scope|
+        run_studio(scope, { dump_only: true })
+        run_clients(scope, { dump_only: true })
+      end
     end
 
     def auto_run
-      run_and_configure_setup
-      begin
+      ResourceScope.open do |scope|
+        run_and_configure_setup scope
         client = @client1
         client.run_tests
         client.create_package
-      ensure
-        @clients&.values&.map(&:abort)
-        @studio&.abort
       end
     end
 

diff --git a/lib/exceptions.rb b/lib/exceptions.rb
@@ -5,6 +5,12 @@ module AutoHCK
   # A custom AutoHCK error exception
   class AutoHCKError < StandardError; end
 
+  # A custom AutoHCK interrupt exception that can be safely blocked with
+  # Thread.handle_interrupt without blocking the other exceptions.
+  # rubocop:disable Lint/InheritException
+  class AutoHCKInterrupt < Exception; end
+  # rubocop:enable Lint/InheritException
+
   # A custom GithubCommitInvalid error exception
   class GithubCommitInvalid < AutoHCKError; end