Skip to content

Commit 9655f0c

Browse files
authored
Fix bug in Fortran dataset destructor (#525)
Fixes a bug in the Fortran client where (on compilers that support automatic destruction) the program would segfault if the user had manually destroyed the dataset. This behaviour was tracked down to an error in the C-client where only the one-level de-referenced pointer was being set to `NULL` but not the actual double-pointer. < committed by @ashao > < reviewed by @al-rigazzi >
1 parent db05610 commit 9655f0c

4 files changed

Lines changed: 19 additions & 4 deletions

File tree

doc/changelog.md

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ To be released at a future time.
66

77
Description
88

9+
- Fix a segfault in the dataset destructor
910
- Update supported Python versions to 3.10, 3.11, and 3.12
1011
- Bump versions for upload/download-artifact Github Actions
1112
- Add Client API functions to put, get, unpack,
@@ -15,8 +16,16 @@ Description
1516
- Reenable move semantics and fix compiler warnings.
1617

1718
Detailed Notes
18-
19-
- Update supported Python versions to 3.10, 3.11, and 3.12
19+
- For compilers that automatically call the dataset
20+
destructor when the object goes out of scope, a segfault
21+
was being triggered if the user was also explictly calling
22+
the destructor. This was partially arising because
23+
although we were freeing the underlying data and pointing to
24+
NULL, the pointer to the object itself was never being
25+
set to NULL. This has been fixed and should now be robust
26+
to multiple calls to the destructor.
27+
([PR525](https://github.com/CrayLabs/SmartRedis/pull/525))
28+
- Update supported Python versions to 3.10, 3.11, and 3.12
2029
([PR527](https://github.com/CrayLabs/SmartRedis/pull/527))
2130
- Bump versions for upload/download-artifact Github Actions
2231
([PR526](https://github.com/CrayLabs/SmartRedis/pull/526))

src/c/c_dataset.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ extern "C" SRError DeallocateDataSet(void** dataset)
9595
DataSet* d = reinterpret_cast<DataSet*>(*dataset);
9696
delete d;
9797
*dataset = NULL;
98+
dataset = NULL;
9899
});
99100
}
100101

src/fortran/dataset.F90

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ subroutine final_destructor(self)
153153
type(dataset_type), intent(inout) :: self
154154
integer :: code
155155

156-
if (c_associated(self%dataset_ptr)) code = dataset_deconstructor(self%dataset_ptr)
156+
code = self%destructor()
157157
end subroutine final_destructor
158158

159159
!> Destroy the dataset

tests/fortran/client_test_dataset.F90

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ program main
7373
integer :: ndims
7474

7575
integer :: i, j, k
76-
type(dataset_type) :: dataset
76+
type(dataset_type) :: dataset, retrieved_dataset
7777
type(client_type) :: client
7878

7979
integer :: err_code
@@ -239,6 +239,11 @@ program main
239239
if (result .ne. SRNoError) error stop
240240
if (.not. exists) error stop 'existent dataset: FAILED'
241241

242+
! Test retrieval of dataset
243+
result = client%get_dataset("test_dataset", retrieved_dataset)
244+
if (result .ne. SRNoError) error stop
245+
if (.not. exists) error stop 'get_dataset: FAILED'
246+
242247

243248
write(*,*) "Fortran Dataset: passed"
244249

0 commit comments

Comments
 (0)