Skip to content

Finalize structures #45

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Finalize structures #45

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb077df
Automatically deallocate strings in testsuite/unittest structures at …
Feb 27, 2025
7ab3524
Automatically deallocate strings in junit_output structure at the end…
Feb 27, 2025
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
52 changes: 52 additions & 0 deletions src/testdrive.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,11 @@ end subroutine test_interface
!> Whether test is supposed to fail
logical :: should_fail = .false.

contains

!> Deallocate unittest's internal data
final :: destroy_unittest

end type unittest_type


Expand All @@ -303,6 +308,11 @@ end subroutine collect_interface
!> Entry point of the test
procedure(collect_interface), pointer, nopass :: collect => null()

contains

!> Deallocate testsuite's internal data
final :: destroy_testsuite

end type testsuite_type


Expand Down Expand Up @@ -334,6 +344,8 @@ end subroutine collect_interface
integer :: skipped = 0
!> Running time
real(sp) :: time = 0.0_sp
contains
final :: destroy_junit_output
end type junit_output


Expand Down Expand Up @@ -764,6 +776,22 @@ subroutine junit_write(junit)
end subroutine junit_write


!> deallocate internal data of junit_output
subroutine destroy_junit_output(self)

!> JUnit output
type(junit_output), intent(inout) :: self

if (allocated(self%xml_start)) deallocate(self%xml_start)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The xml_start string is declared as:

    character(len=:), allocatable :: xml_start

My understanding is that the compiler will always deallocate, unless there is a bug in a compiler.

Did you find a case when this leaks memory? If so, if you could post it, we can create a minimal reproducible example and submit it as a bug to the compiler vendor.

Copy link
Author

@foxtran foxtran Feb 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you find a case when this leaks memory? If so, if you could post it, we can create a minimal reproducible example and submit it as a bug to the compiler vendor.

Yep. Try to compile test-suite with AddressSanitizer with GCC-14.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But is it a bug in GCC or in test-drive? And if it is a bug in GCC, should we workaround the bug in test-drive? We probably should, until they fix it. I'll leave it to @awvwgk to decide.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a bug with finalization in gcc-14, it is hard to draw a mwe but this issue could be related to that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, gcc has several finalization related bugs. (Funny enough, I also found one when developing a unit testing system 😄). Some of them you can work around, and some of them you can not really. In Fortuno, we managed to work around those,so it runs with the address sanitizer without reporting problems.

I think, a testing system should try to work around those issues, because it is somewhat annoying and when the testing framework generates memory leak errors, as if people get used to that, they will just ignore any kind of those errors, even if it is a real bug in their part of the code (and not just compiler artifacts).

if (allocated(self%xml_block)) deallocate(self%xml_block)
if (allocated(self%xml_final)) deallocate(self%xml_final)
if (allocated(self%hostname)) deallocate(self%hostname)
if (allocated(self%package)) deallocate(self%package)
if (allocated(self%testsuite)) deallocate(self%testsuite)

end subroutine destroy_junit_output


!> Create ISO 8601 formatted timestamp
function get_timestamp() result(timestamp)

Expand Down Expand Up @@ -853,6 +881,18 @@ function new_unittest(name, test, should_fail) result(self)
end function new_unittest


!> Finalize unit test
subroutine destroy_unittest(self)

!> unittest to destroy
type(unittest_type), intent(inout) :: self

if (allocated(self%name)) deallocate(self%name)
self%test => null()

end subroutine destroy_unittest


!> Register a new testsuite
function new_testsuite(name, collect) result(self)

Expand All @@ -871,6 +911,18 @@ function new_testsuite(name, collect) result(self)
end function new_testsuite


!> Finalize testsuite
subroutine destroy_testsuite(self)

!> testsuite to destroy
type(testsuite_type), intent(inout) :: self

if (allocated(self%name)) deallocate(self%name)
self%collect => null()

end subroutine destroy_testsuite


subroutine check_stat(error, stat, message, more)

!> Error handling
Expand Down