Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure error message from finalization are displayed to the user #1580

Merged
merged 1 commit into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions scripts/ci-github.sh
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ $run_pip install --upgrade -r requirements.txt
echo Python search paths:
$run_python -c "import sys; print('\n'.join(sys.path))"

echo Check Finalize exception handling :
$run_python ../scripts/python/check_finalize_exceptions.py ../src
echo ............................

echo Running test suite now:
$run_python ./run.py -E || { echo Test suite failures, unstable build!; exit 1; }
cd ..
Expand Down
40 changes: 40 additions & 0 deletions scripts/python/check_finalize_exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#!/usr/bin/env python3
# Find all Finalize procedures that do not report unhandled exceptions

import re
import glob
import sys


def find_matching(source_file, match_pattern, exclude_pattern):
match_re = re.compile(match_pattern)
exclude_re = re.compile(exclude_pattern)

count = 0
with open(source_file, "r", encoding="utf-8") as file:
previous_line = ""
line_nbr = 1
for line in file:
if match_re.search(line) and not exclude_re.search(previous_line):
print(f"{source_file}:{line_nbr}: " + line)
count = count + 1
previous_line = line
line_nbr = line_nbr + 1

return count > 0


if __name__ == "__main__":
if len(sys.argv) != 2:
print("Please provide one directory path")
sys.exit(1)

count = 0
for file in glob.glob(f"{sys.argv[1]}/**/*.ad[bs]"):
if find_matching(
file, ".*end Finalize;.*", ".*Alire.Utils.Finalize_Exception.*"
):
count = count + 1

if count > 0:
sys.exit(1)
11 changes: 3 additions & 8 deletions src/alire/alire-directories.adb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
with AAA.Directories;

with Ada.Exceptions;
with Ada.Numerics.Discrete_Random;
with Ada.Real_Time;
with Ada.Unchecked_Conversion;
Expand All @@ -11,6 +10,7 @@ with Alire.Paths;
with Alire.Platforms.Current;
with Alire.Platforms.Folders;
with Alire.VFS;
with Alire.Utils;

with GNAT.String_Hash;

Expand Down Expand Up @@ -490,7 +490,6 @@ package body Alire.Directories is
overriding
procedure Finalize (This : in out Guard) is
use Ada.Directories;
use Ada.Exceptions;
use Ada.Strings.Unbounded;
procedure Free is
new Ada.Unchecked_Deallocation (Absolute_Path, Destination);
Expand All @@ -506,10 +505,7 @@ package body Alire.Directories is
Free (Freeable);
exception
when E : others =>
Trace.Debug
("FG.Finalize: unexpected exception: " &
Exception_Name (E) & ": " & Exception_Message (E) & " -- " &
Exception_Information (E));
Alire.Utils.Finalize_Exception (E);
end Finalize;

------------------
Expand Down Expand Up @@ -760,8 +756,7 @@ package body Alire.Directories is

exception
when E : others =>
Log_Exception (E);
raise;
Alire.Utils.Finalize_Exception (E);
end Finalize;

--------------------
Expand Down
4 changes: 4 additions & 0 deletions src/alire/alire-errors.adb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
with Ada.Containers.Indefinite_Ordered_Maps;
with Alire.Utils;

package body Alire.Errors is

Expand Down Expand Up @@ -209,6 +210,9 @@ package body Alire.Errors is
pragma Unreferenced (This);
begin
Close;
exception
when E : others =>
Alire.Utils.Finalize_Exception (E);
end Finalize;

-----------
Expand Down
5 changes: 4 additions & 1 deletion src/alire/alire-roots-editable.adb
Original file line number Diff line number Diff line change
Expand Up @@ -523,14 +523,17 @@ package body Alire.Roots.Editable is
Trace.Debug ("Discarding temporary root file: " & File);
end;
end if;
exception
when E : others =>
Alire.Utils.Finalize_Exception (E);
end Finalize;

begin
Finalize (+This.Edit.Manifest);
Finalize (+This.Edit.Lockfile);
exception
when E : others =>
Log_Exception (E, Warning);
Alire.Utils.Finalize_Exception (E);
end Finalize;

---------
Expand Down
3 changes: 3 additions & 0 deletions src/alire/alire-roots.adb
Original file line number Diff line number Diff line change
Expand Up @@ -2020,6 +2020,9 @@ package body Alire.Roots is
(Crate_Configuration.Global_Config, Global_Config_Access);
begin
Free (This.Configuration);
exception
when E : others =>
Alire.Utils.Finalize_Exception (E);
end Finalize;

end Alire.Roots;
6 changes: 6 additions & 0 deletions src/alire/alire-toml_adapters.adb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
with Alire.Utils;

package body Alire.TOML_Adapters is

--------------
Expand All @@ -10,6 +12,10 @@ package body Alire.TOML_Adapters is
begin
-- Manually close this error scope
Errors.Close;

exception
when E : others =>
Alire.Utils.Finalize_Exception (E);
end Finalize;

------------
Expand Down
4 changes: 4 additions & 0 deletions src/alire/alire-utils-text_files.adb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ package body Alire.Utils.Text_Files is
Close (File);
Replacer.Replace;
end;

exception
when E : others =>
Alire.Utils.Finalize_Exception (E);
end Finalize;

-----------
Expand Down
19 changes: 19 additions & 0 deletions src/alire/alire-utils.adb
Original file line number Diff line number Diff line change
Expand Up @@ -253,4 +253,23 @@ package body Alire.Utils is
end if;
end Image_Keys_One_Line;

------------------------
-- Finalize_Exception --
------------------------

procedure Finalize_Exception (E : Ada.Exceptions.Exception_Occurrence) is

-- Import a Last_Chance_Handler procedure that will either be the one
-- declared by Alr, or the default GNAT last chance handler.

procedure Last_Chance_Handler (E : Ada.Exceptions.Exception_Occurrence);
pragma Import (C,
Last_Chance_Handler,
"__gnat_last_chance_handler");
pragma No_Return (Last_Chance_Handler);

begin
Last_Chance_Handler (E);
end Finalize_Exception;

end Alire.Utils;
21 changes: 21 additions & 0 deletions src/alire/alire-utils.ads
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,27 @@ package Alire.Utils with Preelaborate is
-- Flatten String keys of Indefinite_Ordered_Maps into string
-- representation.

procedure Finalize_Exception (E : Ada.Exceptions.Exception_Occurrence);
-- Every controlled object Finalize procedure must call this function to
-- report unhandled exceptions.
--
-- Ada exceptions are not propagated outside the Finalize procedure.
-- Instead, another exception is raise with the message "finalize/adjust
-- raised exception". Alire is using exceptions to report meaningfull error
-- messages to the user. If one of these exception is raised in a Finalize
-- procedure, the error message will vanish and the user will only see
-- "finalize/adjust raised exception".
--
-- For this reason, it is important to catch all exceptions before reaching
-- the end of Finalize and use this Finalize_Exception procedure to display
-- a meaningful error message.
--
-- Use the following code at the end of every Finalize procedures:
-- exception
-- when E : others =>
-- Alire.Utils.Finalize_Exception (E);
-- end Finalize;

private

function Quote (S : String) return String
Expand Down
33 changes: 33 additions & 0 deletions src/alr/alr-commands-dev.adb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
with Ada.Strings.UTF_Encoding.Wide_Wide_Strings;
with Ada.Finalization;

with Alire.Selftest;
with Alire.Utils;

package body Alr.Commands.Dev is

Expand All @@ -24,6 +26,28 @@ package body Alr.Commands.Dev is
Trace.Always (Encode ("ⓘ✓"));
end Print_UTF_8_Sequence;

-----------------------------
-- Raise_From_Finalization --
-----------------------------

procedure Raise_From_Finalization is
type Ctrl is new Ada.Finalization.Controlled with null record;

overriding
procedure Finalize (This : in out Ctrl) is
begin
raise Program_Error with "Raising forcibly from finalization";
exception
when E : others =>
Alire.Utils.Finalize_Exception (E);
end Finalize;

Test : Ctrl;
pragma Unreferenced (Test);
begin
null;
end Raise_From_Finalization;

-------------
-- Execute --
-------------
Expand All @@ -49,6 +73,10 @@ package body Alr.Commands.Dev is
raise Program_Error with "Raising forcibly";
end if;

if Cmd.Raise_Final then
Raise_From_Finalization;
end if;

if Cmd.Self_Test then
Alire.Selftest.Run;
end if;
Expand Down Expand Up @@ -96,6 +124,11 @@ package body Alr.Commands.Dev is
"", "--raise",
"Raise an exception");

Define_Switch (Config,
Cmd.Raise_Final'Access,
"", "--raise-finalization",
"Raise an exception from a finalization procedure");

Define_Switch (Config,
Cmd.Self_Test'Access,
"", "--test",
Expand Down
1 change: 1 addition & 0 deletions src/alr/alr-commands-dev.ads
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ private
Custom : aliased Boolean := False; -- Custom code to run instead
Filtering : aliased Boolean := False; -- Runs debug scope filtering
Raise_Except : aliased Boolean := False;
Raise_Final : aliased Boolean := False;
Self_Test : aliased Boolean := False;
UTF_8_Test : aliased Boolean := False; -- Produce some UTF-8 output
end record;
Expand Down
4 changes: 4 additions & 0 deletions src/alr/alr-utils-temp_file.adb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ package body Alr.Utils.Temp_File is
Delete_File (This.Name);
null;
end if;

exception
when E : others =>
Alire.Utils.Finalize_Exception (E);
end Finalize;

--------------
Expand Down
5 changes: 5 additions & 0 deletions testsuite/tests/debug/enabled-dump-exception/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,9 @@ def check_output(dump):
"ERROR: Raising forcibly\n"
"ERROR: alr encountered an unexpected error, re-run with -d for details.\n")

# Check exception from finalization :
assert_eq(run_alr('dev', '--raise-finalization', debug=False, complain_on_error=False).out,
"ERROR: Raising forcibly from finalization\n"
"ERROR: alr encountered an unexpected error, re-run with -d for details.\n")

print('SUCCESS')
Loading