To prevent Borg hangs, unconditionally delete stale named pipes before dumping databases (#360).

NEWS

@@ -1,6 +1,7 @@
 1.7.3.dev0
  * #357: Add "break-lock" action for removing any repository and cache locks leftover from Borg
    aborting.
+ * #360: To prevent Borg hangs, unconditionally delete stale named pipes before dumping databases.
  * #587: When database hooks are enabled, auto-exclude special files from a "create" action to
    prevent Borg from hanging. You can override/prevent this behavior by explicitly setting the
    "read_special" option to true.

borgmatic/commands/borgmatic.py

@@ -360,7 +360,7 @@ def run_actions(
             **hook_context,
         )
         logger.info('{}: Creating archive{}'.format(repository, dry_run_label))
-        dispatch.call_hooks(
+        dispatch.call_hooks_even_if_unconfigured(
             'remove_database_dumps',
             hooks,
             repository,
@@ -395,7 +395,7 @@ def run_actions(
         if json_output:  # pragma: nocover
             yield json.loads(json_output)
 
-        dispatch.call_hooks(
+        dispatch.call_hooks_even_if_unconfigured(
             'remove_database_dumps',
             hooks,
             config_filename,
@@ -556,7 +556,7 @@ def run_actions(
                     repository, arguments['restore'].archive
                 )
             )
-            dispatch.call_hooks(
+            dispatch.call_hooks_even_if_unconfigured(
                 'remove_database_dumps',
                 hooks,
                 repository,
@@ -626,7 +626,7 @@ def run_actions(
                         extract_process,
                     )
 
-            dispatch.call_hooks(
+            dispatch.call_hooks_even_if_unconfigured(
                 'remove_database_dumps',
                 hooks,
                 repository,

borgmatic/hooks/dispatch.py

@@ -29,19 +29,14 @@ def call_hook(function_name, hooks, log_prefix, hook_name, *args, **kwargs):
     '''
     Given the hooks configuration dict and a prefix to use in log entries, call the requested
     function of the Python module corresponding to the given hook name. Supply that call with the
-    configuration for this hook, the log prefix, and any given args and kwargs. Return any return
+    configuration for this hook (if any), the log prefix, and any given args and kwargs. Return any
-    value.
+    return value.
-
-    If the hook name is not present in the hooks configuration, then bail without calling anything.
 
     Raise ValueError if the hook name is unknown.
     Raise AttributeError if the function name is not found in the module.
     Raise anything else that the called function raises.
     '''
-    config = hooks.get(hook_name)
+    config = hooks.get(hook_name, {})
-    if not config:
-        logger.debug('{}: No {} hook configured.'.format(log_prefix, hook_name))
-        return
 
     try:
         module = HOOK_NAME_TO_MODULE[hook_name]
@@ -59,7 +54,7 @@ def call_hooks(function_name, hooks, log_prefix, hook_names, *args, **kwargs):
     configuration for that hook, the log prefix, and any given args and kwargs. Collect any return
     values into a dict from hook name to return value.
 
-    If the hook name is not present in the hooks configuration, then don't call the function for it,
+    If the hook name is not present in the hooks configuration, then don't call the function for it
     and omit it from the return values.
 
     Raise ValueError if the hook name is unknown.
@@ -71,3 +66,19 @@ def call_hooks(function_name, hooks, log_prefix, hook_names, *args, **kwargs):
         for hook_name in hook_names
         if hooks.get(hook_name)
     }
+
+
+def call_hooks_even_if_unconfigured(function_name, hooks, log_prefix, hook_names, *args, **kwargs):
+    '''
+    Given the hooks configuration dict and a prefix to use in log entries, call the requested
+    function of the Python module corresponding to each given hook name. Supply each call with the
+    configuration for that hook, the log prefix, and any given args and kwargs. Collect any return
+    values into a dict from hook name to return value.
+
+    Raise AttributeError if the function name is not found in the module.
+    Raise anything else that a called function raises. An error stops calls to subsequent functions.
+    '''
+    return {
+        hook_name: call_hook(function_name, hooks, log_prefix, hook_name, *args, **kwargs)
+        for hook_name in hook_names
+    }

borgmatic/hooks/dump.py

@@ -55,7 +55,7 @@ def remove_database_dumps(dump_path, database_type_name, log_prefix, dry_run):
     '''
     dry_run_label = ' (dry run; not actually removing anything)' if dry_run else ''
 
-    logger.info(
+    logger.debug(
         '{}: Removing {} database dumps{}'.format(log_prefix, database_type_name, dry_run_label)
     )
 

tests/unit/commands/test_borgmatic.py

@@ -455,7 +455,8 @@ def test_run_actions_executes_and_calls_hooks_for_create_action():
     flexmock(module.command).should_receive('execute_hook').times(
         4
     )  # Before/after extract and before/after actions.
-    flexmock(module.dispatch).should_receive('call_hooks').and_return({}).times(3)
+    flexmock(module.dispatch).should_receive('call_hooks').and_return({})
+    flexmock(module.dispatch).should_receive('call_hooks_even_if_unconfigured').and_return({})
     arguments = {
         'global': flexmock(monitoring_verbosity=1, dry_run=False),
         'create': flexmock(

tests/unit/hooks/test_dispatch.py

@@ -27,13 +27,18 @@ def test_call_hook_invokes_module_function_with_arguments_and_returns_value():
     assert return_value == expected_return_value
 
 
-def test_call_hook_without_hook_config_skips_call():
+def test_call_hook_without_hook_config_invokes_module_function_with_arguments_and_returns_value():
     hooks = {'other_hook': flexmock()}
+    expected_return_value = flexmock()
     test_module = sys.modules[__name__]
     flexmock(module).HOOK_NAME_TO_MODULE = {'super_hook': test_module}
-    flexmock(test_module).should_receive('hook_function').never()
+    flexmock(test_module).should_receive('hook_function').with_args(
+        {}, 'prefix', 55, value=66
+    ).and_return(expected_return_value).once()
 
-    module.call_hook('hook_function', hooks, 'prefix', 'super_hook', 55, value=66)
+    return_value = module.call_hook('hook_function', hooks, 'prefix', 'super_hook', 55, value=66)
+
+    assert return_value == expected_return_value
 
 
 def test_call_hook_without_corresponding_module_raises():
@@ -76,3 +81,31 @@ def test_call_hooks_calls_skips_return_values_for_null_hooks():
     return_values = module.call_hooks('do_stuff', hooks, 'prefix', ('super_hook', 'other_hook'), 55)
 
     assert return_values == expected_return_values
+
+
+def test_call_hooks_even_if_unconfigured_calls_each_hook_and_collects_return_values():
+    hooks = {'super_hook': flexmock(), 'other_hook': flexmock()}
+    expected_return_values = {'super_hook': flexmock(), 'other_hook': flexmock()}
+    flexmock(module).should_receive('call_hook').and_return(
+        expected_return_values['super_hook']
+    ).and_return(expected_return_values['other_hook'])
+
+    return_values = module.call_hooks_even_if_unconfigured(
+        'do_stuff', hooks, 'prefix', ('super_hook', 'other_hook'), 55
+    )
+
+    assert return_values == expected_return_values
+
+
+def test_call_hooks_even_if_unconfigured_calls_each_hook_configured_or_not_and_collects_return_values():
+    hooks = {'other_hook': flexmock()}
+    expected_return_values = {'super_hook': flexmock(), 'other_hook': flexmock()}
+    flexmock(module).should_receive('call_hook').and_return(
+        expected_return_values['super_hook']
+    ).and_return(expected_return_values['other_hook'])
+
+    return_values = module.call_hooks_even_if_unconfigured(
+        'do_stuff', hooks, 'prefix', ('super_hook', 'other_hook'), 55
+    )
+
+    assert return_values == expected_return_values