silx-kit / h5web

React components for data visualization and exploration
https://h5web.panosc.eu/
MIT License
167 stars 17 forks source link

Fix subtle stale state issue in `H5WasmProvider` #1568

Closed axelboc closed 5 months ago

axelboc commented 5 months ago

We received the following bug report in myHDF5:

I have problem when loading two HDF5 files of different type into the myHDF5 page. If I look at the files one-by-one there is no problem.

Picture (Device Independent Bitmap) 1

When opening the first file then the second file, a bunch of errors are logged to the console that seem to suggest that the entity at the default path is not found due to an incorrect identifier. Important detail: both files have the same default NeXus path.

After some investigation, I identified that the problem comes from a stale state issue in H5WasmProvider: upon receiving new prop values (fileName and buffer), an effect runs in order to create a new API instance, update it in local state, and clean-up the old API instance. However, since this is done asynchronously, the App gets rendered once with the old API instance.

I think the reason why this hadn't caused any issue until now is because it only occurs when switching between two files with the same default path. If the default path of the second file is different, then the old API instance can't find a corresponding entity in the file and stops there silently. On the contrary, if the default path is the same, then the old API instance finds an entity, which means that the viewer then tries to read it... but the internal ID of the entity returned by the old API instance doesn't match the ID of the entity at the same path in the second file, which throws an error.

The difficulty in finding a solution comes from the fact that the API needs to be cleaned up, which typically means useEffect... Unfortunately, the following solutions were not satisfactory:

In the end, I've landed on a solution that involves useMemo and a synchronous setState call. The App is never rendered with a stale API instance because the synchronous setState call tells React to stop rendering the tree; the H5WasmProvider component function is run to completion (so the clean-up call that follows is called as expected), but it stops there and re-renders right away.