Remove double close in pmd_open_series

parcel.h

@@ -1943,7 +1943,7 @@ static pmd_status read_series_metadata_from_file(hid_t file_id, pmd_series *seri
     if (status != PMD_SUCCESS) {
         return status;
     }
-    /* Parse iteration encoding and handle file lifecycle */
+    /* Parse iteration encoding (file_id is owned by the caller and is not closed here) */
     if (strcmp(iter_encoding_str, "fileBased") == 0) {
         series->iteration_encoding = PMD_FILE_BASED;
 
@@ -1962,10 +1962,6 @@ static pmd_status read_series_metadata_from_file(hid_t file_id, pmd_series *seri
             return PMD_ERROR_FILE_FORMAT;
         }
         free_iteration_pattern(&pattern_check);
-
-        /* Don't keep file open for fileBased */
-        H5Fclose(file_id);
-        series->file_id = -1;
     } else if (strcmp(iter_encoding_str, "groupBased") == 0) {
         series->iteration_encoding = PMD_GROUP_BASED;
 
@@ -1974,9 +1970,6 @@ static pmd_status read_series_metadata_from_file(hid_t file_id, pmd_series *seri
             free(iter_encoding_str);
             return PMD_ERROR_FILE_FORMAT;
         }
-
-        /* For groupBased, keep file open */
-        series->file_id = file_id;
     } else {
         free(iter_encoding_str);
         return PMD_ERROR_FILE_FORMAT;
@@ -2344,6 +2337,14 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac
                     }
                     maybe_padded_file = maybe_padded_iter_filename;
 
+                    /* A %T filename pattern is only valid for file-based series */
+                    if (series->iteration_encoding != PMD_FILE_BASED) {
+                        pmd_log(PMD_LOG_ERROR, "File '%s' matched pattern '%s' but is not a fileBased series",
+                                maybe_padded_iter_filename, filename);
+                        status = PMD_ERROR_FILE_FORMAT;
+                        goto cleanup;
+                    }
+
                     /* Close file since we will not store for file-based mode */
                     H5Fclose(file_id);
                     file_id = -1;
@@ -2383,8 +2384,9 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac
             /* Create base path structure up to scan_parent for GROUP_BASED */
             status = ensure_base_path_groups(file_id, series->base_path);
 
-            /* Keep file open for group-based */
+            /* Keep file open for group-based; series now owns the handle */
             series->file_id = file_id;
+            file_id = -1;
         }
         /* If we are opening an existing file (must exist) */
         else{
@@ -2411,14 +2413,21 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac
                 status = PMD_ERROR_HDF5;
                 goto cleanup;
             }
-            series->file_id = file_id;
 
             /* Read metadata from the opened file */
             status = read_series_metadata_from_file(file_id, series, filename);
             if (status != PMD_SUCCESS) {
                 goto cleanup;
             }
             maybe_padded_file = filename;
+
+            /* Group-based series keep the file open; file-based series open iteration files on demand */
+            if (series->iteration_encoding == PMD_GROUP_BASED) {
+                series->file_id = file_id;
+            } else {
+                H5Fclose(file_id);
+            }
+            file_id = -1;
         }
 
         /* For FILE_BASED series, set directory to parent of filename */
@@ -2449,7 +2458,7 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac
     }
 
 cleanup:
-    if (file_id >= 0 && series->file_id < 0) {
+    if (file_id >= 0) {
         H5Fclose(file_id);
     }
     if (status != PMD_SUCCESS) {

tests/test_read.c

@@ -427,6 +427,52 @@ void test_file_based_multiple_percent_t(void) {
     pmd_close_series(series);
 }
 
+static herr_t count_hdf5_errors(hid_t estack, void *client_data) {
+    (void)estack;
+    (*(int *)client_data)++;
+    return 0;
+}
+
+/* Test: Opening and closing series releases each HDF5 file handle exactly once
+ * Files: tests/data/file_based_series/data_{0,%T}.h5, file_based_iteration_format_mismatch.h5,
+ *        valid_multiple_iterations.h5
+ * Tests: No HDF5 errors (e.g. closing an already closed file) and no leaked file handles */
+void test_open_series_no_double_close(void) {
+    pmd_series *series;
+    int hdf5_error_count = 0;
+    ssize_t open_files_before = H5Fget_obj_count((hid_t)H5F_OBJ_ALL, H5F_OBJ_FILE);
+
+    /* Count HDF5 errors instead of silencing them; assertions run after the handler is restored */
+    H5Eset_auto2(H5E_DEFAULT, count_hdf5_errors, &hdf5_error_count);
+
+    /* FILE_BASED, specific file */
+    pmd_status file_based_result = pmd_open_series("tests/data/file_based_series/data_0.h5", &series, PMD_RDONLY);
+    pmd_close_series(series);
+
+    /* FILE_BASED, %T pattern */
+    pmd_status pattern_result = pmd_open_series("tests/data/file_based_series/data_%T.h5", &series, PMD_RDONLY);
+    pmd_close_series(series);
+
+    /* FILE_BASED, error after metadata has been read */
+    pmd_status mismatch_result = pmd_open_series("tests/data/file_based_iteration_format_mismatch.h5",
+                                                 &series, PMD_RDONLY);
+    pmd_close_series(series);
+
+    /* GROUP_BASED, series keeps the file open */
+    pmd_status group_based_result = pmd_open_series("tests/data/valid_multiple_iterations.h5", &series, PMD_RDONLY);
+    pmd_close_series(series);
+
+    ssize_t open_files_after = H5Fget_obj_count((hid_t)H5F_OBJ_ALL, H5F_OBJ_FILE);
+    H5Eset_auto2(H5E_DEFAULT, NULL, NULL);
+
+    TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, file_based_result);
+    TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, pattern_result);
+    TEST_ASSERT_EQUAL_INT(PMD_ERROR_FILE_FORMAT, mismatch_result);
+    TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, group_based_result);
+    TEST_ASSERT_EQUAL_INT_MESSAGE(0, hdf5_error_count, "HDF5 reported errors while opening/closing series");
+    TEST_ASSERT_EQUAL_INT64((int64_t)open_files_before, (int64_t)open_files_after);
+}
+
 /* Test: Group-based series with multiple iterations
  * File: tests/data/valid_multiple_iterations.h5
  * Tests: Basic group-based iteration enumeration */
@@ -3045,6 +3091,7 @@ int main(void) {
     RUN_TEST(test_file_based_series_pattern_path);
     RUN_TEST(test_file_based_series_with_other_files);
     RUN_TEST(test_file_based_multiple_percent_t);
+    RUN_TEST(test_open_series_no_double_close);
     RUN_TEST(test_group_based_series_multiple_iterations);
     RUN_TEST(test_group_based_non_matching_groups);
     RUN_TEST(test_iteration_format_prefix_suffix);

tests/test_write.c

@@ -1009,6 +1009,28 @@ void test_invalid_pattern_ambiguous(void) {
     TEST_ASSERT_NOT_EQUAL(PMD_SUCCESS, result);
 }
 
+/**
+ * Test: %T filename pattern that matches a group-based file fails
+ */
+void test_pattern_matching_group_based_file_fails(void) {
+    pmd_series *series;
+    pmd_status result;
+
+    /* Create a group-based file whose name matches grp_%T.h5 */
+    result = pmd_open_series(TEST_TEMP_DIR "/grp_0.h5", &series, PMD_TRUNC);
+    TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result);
+    pmd_close_series(series);
+
+    ssize_t open_files_before = H5Fget_obj_count((hid_t)H5F_OBJ_ALL, H5F_OBJ_FILE);
+
+    result = pmd_open_series(TEST_TEMP_DIR "/grp_%T.h5", &series, PMD_RDONLY);
+    TEST_ASSERT_EQUAL_INT(PMD_ERROR_FILE_FORMAT, result);
+    TEST_ASSERT_NULL(series);
+
+    ssize_t open_files_after = H5Fget_obj_count((hid_t)H5F_OBJ_ALL, H5F_OBJ_FILE);
+    TEST_ASSERT_EQUAL_INT64((int64_t)open_files_before, (int64_t)open_files_after);
+}
+
 /**
  * Test: Various valid file-based iteration patterns
  * Tests multiple pattern formats in a parameterized style
@@ -2708,6 +2730,7 @@ int main(void) {
     RUN_TEST(test_write_nonconsecutive_iterations_file_based);
     RUN_TEST(test_write_fails_no_parent_directory);
     RUN_TEST(test_invalid_pattern_ambiguous);
+    RUN_TEST(test_pattern_matching_group_based_file_fails);
     RUN_TEST(test_valid_filebased_patterns);
     RUN_TEST(test_truncate_deletes_existing_files);
     RUN_TEST(test_filebased_fails_parent_before_t_missing);