| Message ID | 20241221-b4-pks-reftable-oom-fix-without-readers-v1-2-12db83a3267c@pks.im (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | reftable: fix out-of-memory errors on NonStop | expand |
Hi On Sat, Dec 21, 2024, at 12:50, Patrick Steinhardt wrote: > It was reported [1c A Neo user, I see. s/c/]/ > [snip] > the result. Now that we do we notice and thus return an out-of-memory > error to the caller. s/Now that we do we/Now we do notice/ Repeat of “we”. And “that” can’t be be used since it has nothing to connect to.
On Sat, Dec 21, 2024 at 01:40:32PM +0100, Kristoffer Haugsbakk wrote: > Hi > > On Sat, Dec 21, 2024, at 12:50, Patrick Steinhardt wrote: > > It was reported [1c > > A Neo user, I see. There are dozens of us! > s/c/]/ Oh, I even saw that typo, but obviously forgot to fix it. > > [snip] > > the result. Now that we do we notice and thus return an out-of-memory > > error to the caller. > > s/Now that we do we/Now we do notice/ > > Repeat of “we”. And “that” can’t be be used since it has nothing to > connect to. Indeed. Will wait a bit to hear back from Randall before sending another version. Thanks! Patrick
"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes: > Hi > > On Sat, Dec 21, 2024, at 12:50, Patrick Steinhardt wrote: >> It was reported [1c > > A Neo user, I see. > > s/c/]/ > >> [snip] >> the result. Now that we do we notice and thus return an out-of-memory >> error to the caller. > > s/Now that we do we/Now we do notice/ > > Repeat of “we”. And “that” can’t be be used since it has nothing to > connect to. I read "Now that we do, we notice and ..." a perfectly sensible construct, though. Thanks.
diff --git a/reftable/merged.c b/reftable/merged.c index bb0836e3443271f9c0d5ba5582c78694d437ddc2..e72b39e178d4dec6ddfca970b5af71b71431397a 100644 --- a/reftable/merged.c +++ b/reftable/merged.c @@ -240,14 +240,16 @@ int merged_table_init_iter(struct reftable_merged_table *mt, struct reftable_iterator *it, uint8_t typ) { - struct merged_subiter *subiters; + struct merged_subiter *subiters = NULL; struct merged_iter *mi = NULL; int ret; - REFTABLE_CALLOC_ARRAY(subiters, mt->readers_len); - if (!subiters) { - ret = REFTABLE_OUT_OF_MEMORY_ERROR; - goto out; + if (mt->readers_len) { + REFTABLE_CALLOC_ARRAY(subiters, mt->readers_len); + if (!subiters) { + ret = REFTABLE_OUT_OF_MEMORY_ERROR; + goto out; + } } for (size_t i = 0; i < mt->readers_len; i++) {
It was reported [1c that Git started to fail with an out-of-memory error when initializing repositories with the reftable backend on NonStop platforms. A bisect led to 802c0646ac (reftable/merged: handle allocation failures in `merged_table_init_iter()`, 2024-10-02), which changed how we allocate memory when initializing a merged table. The root cause of this seems to be that NonStop returns a `NULL` pointer when doing a zero-sized allocation. This would've already happened before the above change, but we never noticed because we did not check the result. Now that we do we notice and thus return an out-of-memory error to the caller. Fix the issue by skipping the allocation altogether in case there are no readers. [1]: <00ad01db5017$aa9ce340$ffd6a9c0$@nexbridge.com> Reported-by: Randall S. Becker <rsbecker@nexbridge.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> --- reftable/merged.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)