diff --git a/src/mbio/mb_mem.c b/src/mbio/mb_mem.c index 852689b13..78bac01d3 100644 --- a/src/mbio/mb_mem.c +++ b/src/mbio/mb_mem.c @@ -931,6 +931,12 @@ int mb_update_arrays(int verbose, void *mbio_ptr, int nbath, int namp, int nss, /* get mbio descriptor */ struct mb_io_struct *mb_io_ptr = (struct mb_io_struct *)mbio_ptr; + /* Discard pointer values from earlier reallocation cycles so allocator + address reuse cannot cause a false match in mb_update_arrayptr(). */ + for (int i = 0; i < mb_io_ptr->n_regarray; i++) { + mb_io_ptr->regarray_oldptr[i] = NULL; + } + /* reallocate larger arrays if necessary */ int status = MB_SUCCESS; if (nbath > mb_io_ptr->beams_bath_alloc) { @@ -1170,12 +1176,28 @@ int mb_update_arrayptr(int verbose, void *mbio_ptr, void **handle, int *error) { /* get mbio descriptor */ struct mb_io_struct *mb_io_ptr = (struct mb_io_struct *)mbio_ptr; - /* look for handle in registered arrays */ + /* Prefer the registered pointer variable's address, which remains stable + when its allocation is resized. */ bool found = false; - for (int i = 0; i < mb_io_ptr->n_regarray && !found; i++) { - if (*handle == mb_io_ptr->regarray_oldptr[i]) { + for (int i = 0; i < mb_io_ptr->n_regarray; i++) { + if ((void *)handle == mb_io_ptr->regarray_handle[i]) { *handle = mb_io_ptr->regarray_ptr[i]; + mb_io_ptr->regarray_oldptr[i] = NULL; found = true; + break; + } + } + + /* The read APIs call this function with addresses of local pointer copies, + which do not match regarray_handle[]. Fall back to the pointer value saved + by the current mb_update_arrays() call. */ + if (!found) { + for (int i = 0; i < mb_io_ptr->n_regarray; i++) { + if (mb_io_ptr->regarray_oldptr[i] != NULL && *handle == mb_io_ptr->regarray_oldptr[i]) { + *handle = mb_io_ptr->regarray_ptr[i]; + mb_io_ptr->regarray_oldptr[i] = NULL; + break; + } } } diff --git a/test/mbio/mb_mem_test.cc b/test/mbio/mb_mem_test.cc index 9eb3f10b7..d92eb81bf 100644 --- a/test/mbio/mb_mem_test.cc +++ b/test/mbio/mb_mem_test.cc @@ -4,8 +4,10 @@ #include #include +#include #include "mb_define.h" +#include "mb_io.h" #include "mb_status.h" #include @@ -97,10 +99,108 @@ TEST(MbDebug, FreeNullptr) { // TODO(schwehr): Test mb_memory_clear // TODO(schwehr): Test mb_memory_status // TODO(schwehr): Test mb_memory_list -// TODO(schwehr): Test mb_register_array -// TODO(schwehr): Test mb_update_arrays -// TODO(schwehr): Test mb_update_arrayptr // TODO(schwehr): Test mb_list_arrays -// TODO(schwehr): Test mb_deall_ioarrays + +void InitMinimalMbio(struct mb_io_struct *mbio, int beams, int pixels) { + memset(mbio, 0, sizeof(*mbio)); + mbio->beams_bath_max = beams; + mbio->beams_amp_max = beams; + mbio->pixels_ss_max = pixels; + mbio->beams_bath_alloc = beams; + mbio->beams_amp_alloc = beams; + mbio->pixels_ss_alloc = pixels; +} + +// Simulate mb_read() after bathymetry and sidescan arrays grow in one cycle; +// each local pointer copy must be rebound to its registered allocation. +TEST(MbMem, UpdateArrayptrLocalCopyAfterBathAndSidescanGrow) { + int error = MB_ERROR_NO_ERROR; + const int verbose = 0; + struct mb_io_struct mbio; + InitMinimalMbio(&mbio, 254, 8192); + + double *bath = nullptr; + double *sslon = nullptr; + ASSERT_EQ(MB_SUCCESS, mb_register_array(verbose, &mbio, MB_MEM_TYPE_BATHYMETRY, + sizeof(double), (void **)&bath, &error)); + ASSERT_EQ(MB_SUCCESS, mb_register_array(verbose, &mbio, MB_MEM_TYPE_SIDESCAN, + sizeof(double), (void **)&sslon, &error)); + ASSERT_NE(bath, nullptr); + ASSERT_NE(sslon, nullptr); + ASSERT_NE(bath, sslon); + + double *bath_local = bath; + double *sslon_local = sslon; + + ASSERT_EQ(MB_SUCCESS, mb_update_arrays(verbose, &mbio, 800, 800, 9664, &error)); + ASSERT_EQ(MB_SUCCESS, mb_update_arrayptr(verbose, &mbio, (void **)&bath_local, &error)); + ASSERT_EQ(MB_SUCCESS, mb_update_arrayptr(verbose, &mbio, (void **)&sslon_local, &error)); + + EXPECT_EQ(bath_local, bath); + EXPECT_EQ(sslon_local, sslon); + EXPECT_NE(bath_local, sslon_local); + + sslon_local[mbio.pixels_ss_max - 1] = 42.0; + EXPECT_EQ(sslon_local[mbio.pixels_ss_max - 1], 42.0); + + EXPECT_EQ(MB_SUCCESS, mb_deall_ioarrays(verbose, &mbio, &error)); +} + +TEST(MbMem, UpdateArrayptrMatchesHandleNotStaleOldptr) { + int error = MB_ERROR_NO_ERROR; + const int verbose = 0; + struct mb_io_struct mbio; + InitMinimalMbio(&mbio, 254, 8192); + + double *bath = nullptr; + double *sslon = nullptr; + ASSERT_EQ(MB_SUCCESS, mb_register_array(verbose, &mbio, MB_MEM_TYPE_BATHYMETRY, + sizeof(double), (void **)&bath, &error)); + ASSERT_EQ(MB_SUCCESS, mb_register_array(verbose, &mbio, MB_MEM_TYPE_SIDESCAN, + sizeof(double), (void **)&sslon, &error)); + + double *const sslon_allocation = sslon; + double *const bath_allocation = bath; + mbio.regarray_oldptr[0] = sslon; + + ASSERT_EQ(MB_SUCCESS, mb_update_arrayptr(verbose, &mbio, (void **)&sslon, &error)); + EXPECT_EQ(sslon, sslon_allocation); + EXPECT_NE(sslon, bath_allocation); + EXPECT_EQ(sslon, mbio.regarray_ptr[1]); + EXPECT_EQ(mbio.regarray_oldptr[0], sslon_allocation); + + EXPECT_EQ(MB_SUCCESS, mb_deall_ioarrays(verbose, &mbio, &error)); +} + +TEST(MbMem, UpdateArrayptrLocalCopyAfterStagedBathThenSidescanGrow) { + int error = MB_ERROR_NO_ERROR; + const int verbose = 0; + struct mb_io_struct mbio; + InitMinimalMbio(&mbio, 254, 8192); + + double *bath = nullptr; + double *sslon = nullptr; + ASSERT_EQ(MB_SUCCESS, mb_register_array(verbose, &mbio, MB_MEM_TYPE_BATHYMETRY, + sizeof(double), (void **)&bath, &error)); + ASSERT_EQ(MB_SUCCESS, mb_register_array(verbose, &mbio, MB_MEM_TYPE_SIDESCAN, + sizeof(double), (void **)&sslon, &error)); + + double *bath_local = bath; + ASSERT_EQ(MB_SUCCESS, mb_update_arrays(verbose, &mbio, 800, 254, 8192, &error)); + ASSERT_EQ(MB_SUCCESS, mb_update_arrayptr(verbose, &mbio, (void **)&bath_local, &error)); + EXPECT_EQ(bath_local, bath); + + // Simulate allocator reuse: the stale bathymetry address now identifies the + // current sidescan allocation. + mbio.regarray_oldptr[0] = sslon; + double *sslon_local = sslon; + ASSERT_EQ(MB_SUCCESS, mb_update_arrays(verbose, &mbio, 800, 800, 9664, &error)); + EXPECT_EQ(mbio.regarray_oldptr[0], nullptr); + ASSERT_EQ(MB_SUCCESS, mb_update_arrayptr(verbose, &mbio, (void **)&sslon_local, &error)); + EXPECT_EQ(sslon_local, sslon); + EXPECT_NE(sslon_local, bath_local); + + EXPECT_EQ(MB_SUCCESS, mb_deall_ioarrays(verbose, &mbio, &error)); +} } // namespace