| Message ID | 20241221-b4-pks-reftable-oom-fix-without-readers-v1-0-12db83a3267c@pks.im (mailing list archive) |
|---|---|
| Headers | show |
| Series | reftable: fix out-of-memory errors on NonStop | expand |
On December 21, 2024 6:50 AM, Patrick Steinhardt wrote: >this small patch series fixes out-of-memory errors on NonStop with the reftable >backend. These errors are caused by zero-sized allocations, which return `NULL` >pointers on NonStop. > >Thanks! > >Patrick > >--- >Patrick Steinhardt (4): > reftable/stack: don't perform auto-compaction with less than two tables > reftable/merged: fix zero-sized allocation when there are no readers > reftable/stack: fix zero-sized allocation when there are no readers > reftable/basics: return NULL on zero-sized allocations > > reftable/basics.c | 7 +++++++ > reftable/merged.c | 12 +++++++----- > reftable/stack.c | 47 ++++++++++++++++++++++++++--------------------- > 3 files changed, 40 insertions(+), 26 deletions(-) > > >--- >base-commit: ff795a5c5ed2e2d07c688c217a615d89e3f5733b >change-id: 20241220-b4-pks-reftable-oom-fix-without-readers-c7d8fda0694d From the malloc man page: "If size is 0, then malloc() returns either NULL, or a unique pointer value that can later be successfully passed to free()." Thank you for this series. I think the problem may not be limited only to NonStop based on the documented ambiguous behaviour of malloc. --Randall
Patrick Steinhardt <ps@pks.im> writes: > this small patch series fixes out-of-memory errors on NonStop with the > reftable backend. These errors are caused by zero-sized allocations, > which return `NULL` pointers on NonStop. Interesting. Git side has long been aware of this issue, but because being able to get something out of zero-byte allocation was somehow deemed so useful (and I do not either remember the reasons, nor if I were asked today, I do not know if I agree), our xmalloc() and friends ensure that we never return NULL for 0-sized allocation. I looked at the patches the places they touch and they are sensible changes to avoid asking 0-sized allocations. Which is the opposite approach from what Git takes *and* which may be more sensible. The callers should know better what they want to do when they have zero sized request. E.g., if they have 0 records to write to in a format that has num-records followed by an array of entries, they may want to make it a no-op, or they may want to explicitly write 0 followed by no entries, to record the fact that such a request was made. One possible downside of the approach is that all API functions must be vetted for their response to a request that might end up leading to a zero-sized allocation and told to special case such a request, but that is something better donw sooner rather than later. Will queue. Thanks.
Randall Becker <randall.becker@nexbridge.ca> writes: > Thank you for this series. I think the problem may not be limited > only to NonStop based on the documented ambiguous behaviour of > malloc. Yes, an implementation is allowed to return NULL when asked to allocate 0 bytes. On the Git side, we sidestep the problem in exactly the opposite way---our malloc() wrapper makes another request to allocate 1-byte block after seeing NULL from a request for 0-byte allocation. The reftable code takes an approach to eliminate the need to requiest 0-sized allocation in the first place, which is an arguably more valid solution. Thanks, both, for finding and fixing the issue.
Hi, this small patch series fixes out-of-memory errors on NonStop with the reftable backend. These errors are caused by zero-sized allocations, which return `NULL` pointers on NonStop. Thanks! Patrick --- Patrick Steinhardt (4): reftable/stack: don't perform auto-compaction with less than two tables reftable/merged: fix zero-sized allocation when there are no readers reftable/stack: fix zero-sized allocation when there are no readers reftable/basics: return NULL on zero-sized allocations reftable/basics.c | 7 +++++++ reftable/merged.c | 12 +++++++----- reftable/stack.c | 47 ++++++++++++++++++++++++++--------------------- 3 files changed, 40 insertions(+), 26 deletions(-) --- base-commit: ff795a5c5ed2e2d07c688c217a615d89e3f5733b change-id: 20241220-b4-pks-reftable-oom-fix-without-readers-c7d8fda0694d