HDFGroup / HDFGroup/hdf5

H5DSattach_scale() leaks a dataset ID per existing reference, keeping the file open (pass-through VOL only)

Open
#6,639 0 comments 0 reactions 1 assignee Claimed by @hyoklee View on GitHub
Component - C Library
Dominant language
C
Stars
988
Forks
355
Avg merge
4d 2h
Merged PRs (30d)
12

Description

### Summary

`H5DSattach_scale()` leaks one dataset identifier per reference already stored
in the scale's `REFERENCE_LIST` attribute. Each leaked identifier keeps its file
open, so a later `H5Fcreate(..., H5F_ACC_TRUNC, ...)` on the same file fails
with *"unable to truncate a file which is already open"* — an error with nothing
in it to point back at a dimension scale.

Only reachable through a pass-through VOL connector (see "Why it is normally
invisible" below).

### The code

`hl/src/H5DS.c`, in the loop that recreates the references read from the
existing `REFERENCE_LIST`:

```c
for (j = 0; j < nelmts - 1; j++) {
if (is_new_ref) {
ndsbuf_w[j].dim_idx = ndsbuf[j].dim_idx;
tmp_id = H5Ropen_object(&ndsbuf[j].ref, H5P_DEFAULT, H5P_DEFAULT);
if (tmp_id < 0)
goto out;
if (H5Rcreate_object(tmp_id, ".", H5P_DEFAULT, &ndsbuf_w[j].ref) < 0) {
H5Dclose(tmp_id); /* <-- error path closes it */
goto out;
}
/* <-- success path does not */
}
else {
dsbuf_w[j] = dsbuf[j];
}
}
```

`tmp_id` is closed only when `H5Rcreate_object()` fails. On success it is
dropped. The identical loop in `H5DSdetach_scale()` has always had the closing
`H5Dclose(tmp_id)` after the `H5Rcreate_object()` call, so this looks like a
straightforward omission rather than a deliberate difference.

### Why it is normally invisible

`H5DSwith_new_ref()` sets

```c
*with_new_ref = (config_flag || !native);
```

so with the native VOL, and without `H5_DIMENSION_SCALES_WITH_NEW_REF`,
`is_new_ref` is false and the `else` branch runs — which opens nothing. The
leaking branch is reached only by a **pass-through VOL connector**, where
`H5VLobject_is_native()` reports false.

### Reproducer

Build against `develop` and run under any pass-through connector. This uses the
library's own `pass_through`, forced non-native by hand; in the original report
it was a third-party pass-through connector.

```c
#include
#include
#include
#define F "rep.h5"

static void rep(const char *t) {
printf("%-22s file ids = %zd, dataset ids = %zd\n", t,
(ssize_t)H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_FILE),
(ssize_t)H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_DATASET));
}

int main(void) {
hsize_t dsd[1] = {3}, dims[2] = {3, 2};
hid_t fid = H5Fcreate(F, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT);
hid_t gid = H5Gcreate1(fid, "g", 0);
hid_t dss = H5Screate_simple(1, dsd, dsd);
hid_t scale = H5Dcreate1(gid, "dimscale", H5T_NATIVE_INT, dss, H5P_DEFAULT);
H5DSset_scale(scale, "name");

hid_t s1 = H5Screate_simple(1, dims, dims);
hid_t v1 = H5Dcreate1(gid, "var1", H5T_NATIVE_INT, s1, H5P_DEFAULT);
H5DSattach_scale(v1, scale, 0); /* REFERENCE_LIST is empty here */

hid_t s3 = H5Screate_simple(2, dims, dims);
hid_t v3 = H5Dcreate1(gid, "var3", H5T_NATIVE_INT, s3, H5P_DEFAULT);
H5DSattach_scale(v3, scale, 0); /* one existing reference -> leak */
rep("after 2nd attach");

H5Dclose(scale); H5Dclose(v1); H5Dclose(v3);
H5Sclose(dss); H5Sclose(s1); H5Sclose(s3); H5Gclose(gid); H5Fclose(fid);
rep("after H5Fclose");

fid = H5Fcreate(F, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT);
printf("recreate = %lld\n", (long long)fid);
return fid < 0;
}
```

Native VOL:

```
after 2nd attach file ids = 1, dataset ids = 4
after H5Fclose file ids = 0, dataset ids = 0
recreate = 72057594037927937
```

Pass-through connector:

```
after 2nd attach file ids = 1, dataset ids = 5 <-- one extra
after H5Fclose file ids = 1, dataset ids = 0 <-- file still open
HDF5-DIAG: Error detected in HDF5 (2.3.0):
#003: H5Fint.c line 1949 in H5F_open(): unable to truncate a file which is already open
recreate = -1
```

The extra dataset identifier is `tmp_id`; the still-open file identifier is what
it holds.

### How it was found

Running the netCDF-C test suite over a pass-through VOL connector.
`h5_test/tst_h_dimscales` builds a dimension-scale file, reads it back, then
re-creates the same file with `H5F_ACC_TRUNC` and fails there.

### Fix

Close `tmp_id` on the success path too, as `H5DSdetach_scale()` does. PR to
follow.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.