Skip to content

Fix mb_update_arrayptr() false matches after array reallocation #1

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.

Sign up for GitHub

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

Open
spatial-dan wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix mb_update_arrayptr() false matches after array reallocation #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
818874b
Fix mb_update_arrayptr() false match after array reallocation.
Aug 29, 2026
a10f09e
Strengthen mb_update_arrayptr() regression test.
Aug 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions src/mbio/mb_mem.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
}
}

Expand Down
108 changes: 104 additions & 4 deletions test/mbio/mb_mem_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

#include <cstdio>
#include <cstdlib>
#include <cstring>

#include "mb_define.h"
#include "mb_io.h"
#include "mb_status.h"

#include <gmock/gmock.h>
Expand Down Expand Up @@ -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