-
Notifications
You must be signed in to change notification settings - Fork 263
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
Add H5FD_http_finalize function and call on hdf5 finalize #2827
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think this is correct. You are replacing H5FD_http_term with H5FD_http_finalize (BTW why the name change?). But there are other occurrences of H5FD_http_term that you missed e.g. near line 173 of H5FDhttp.c.
H5FD_http_finalize is added to unregister the class - the terminate call in the class structure (H5FD_http_term) is still there and potentially called from within the class, just it doesnt need to do anything in this case It looks like the terminate call is called as class shutsdown, which is normally when HDF5 library closes. In a case where netcdf AND hdf5 are closing at the same time, this is ok. In the case like octave where the H5 library is still around after netcdf has closed, it is a problem as the h5 library still has references to classes registered from netcdf. Hence the new function intended on just unregistering the class that was registered via H5FD_http_init |
But wouldn't it work to add the unregister to H5fD_http_term? |
Possibly? But you would also need to call H5fD_http_term from within hdf5dispatch and provide it as an external function within libhdf5/H5FDhttp.h That also assumes that calling unregister from that function when its being as terminate() on a shutdown in the class is also ok. |
I did a quick test and calling unregister from H5FD_http_term causes a crash unless it is removed from being a reference in the H5FD_http_g struct. If you would rather keep the function as that and call it from dispatch, but remove from the class as terminate, I can change it |
So going forward, I should call unregister from H5fD_http_term, call H5fD_http_term from within hdf5dispatch, remove H5fD_http_term from being referenced in the H5FD_http_g class and remove H5FD_http_finalize ? |
Fixes #2617