fix: handle OOM correctly in parser and DOM code

benchmark/sonic.hpp

@@ -22,9 +22,7 @@
 template <typename NodeType>
 class SonicStringResult : public StringResult<SonicStringResult<NodeType>> {
  public:
-  std::string_view str_impl() const {
-    return const_cast<sonic_json::WriteBuffer &>(wb).ToString();
-  }
+  std::string_view str_impl() const { return wb.ToString(); }
   sonic_json::WriteBuffer wb;
 };
 

docs/usage.md

@@ -45,6 +45,10 @@ Document is the manager of Nodes. Sonic-Cpp organizes JSON value as a tree.
 Document also the root of JSON value tree. There is an allocator in Document,
 which you should use to allocate memory for Node and Document.
 
+> **Note:** Re-parsing a `Document` discards the previous tree. Any raw
+> pointers, iterators, or `DNode*` obtained from an earlier `Parse()` become
+> invalid and must be re-acquired after each parse.
+
 ### Query in object
 There are two ways to find members: `operator[]` or `FindMember`. We recommend
 using `FindMember`.
@@ -209,6 +213,23 @@ using MyDoc = sonic_json::GenericDocument<MyNode>;
 Sonic uses rapidjson's allocator, you can define your own allocator follow
 [rapidjson allocaotr](http://rapidjson.org/md_doc_internals.html#InternalAllocator)
 
+### Detecting OOM on Post-Parse Mutations
+
+DNode mutations like `PushBack`, `AddMember`, and `Reserve` do not return a
+status code. When you use `MemoryPoolAllocator`, you can check
+`HadOom()` / `ClearOom()` around these operations if you need to detect an
+allocation failure:
+
+```c++
+auto& alloc = doc.GetAllocator();
+alloc.ClearOom();
+doc.PushBack(v, alloc);
+if (alloc.HadOom()) { /* handle OOM */ }
+```
+
+The flag is sticky until cleared. This is a `MemoryPoolAllocator` feature, not
+part of the abstract allocator concept.
+
 ### JSON Pointer
 Sonic provides a JsonPointer class but doesn't support resolving the JSON pointer
 syntax of [RFC 6901](https://www.rfc-editor.org/rfc/rfc6901). We will support

include/sonic/allocator.h

@@ -190,6 +190,9 @@ class MemoryPoolAllocator {
         ownBaseAllocator;  //!< base allocator created by this object.
     size_t refcount;
     bool ownBuffer;
+    //!< Sticky OOM flag shared across refcounted copies.  Atomic because
+    //!< the per-instance SpinLock does not synchronize different copies.
+    std::atomic<bool> hadOom;
   };
 
   static const size_t SIZEOF_SHARED_DATA = SONIC_ALIGN(sizeof(SharedData));
@@ -226,6 +229,7 @@ class MemoryPoolAllocator {
                            : 0)) {
     sonic_assert(baseAllocator_ != 0);
     sonic_assert(shared_ != 0);
+    new (&shared_->hadOom) std::atomic<bool>(false);
     if (baseAllocator) {
       shared_->ownBaseAllocator = 0;
     } else {
@@ -258,12 +262,13 @@ class MemoryPoolAllocator {
         baseAllocator_(baseAllocator ? baseAllocator : new BaseAllocator()),
         shared_(static_cast<SharedData*>(AlignBuffer(buffer, size))) {
     sonic_assert(size >= SIZEOF_SHARED_DATA + SIZEOF_CHUNK_HEADER);
+    new (&shared_->hadOom) std::atomic<bool>(false);
     shared_->chunkHead = GetChunkHead(shared_);
     shared_->chunkHead->capacity =
         size - SIZEOF_SHARED_DATA - SIZEOF_CHUNK_HEADER;
     shared_->chunkHead->size = 0;
     shared_->chunkHead->next = 0;
-    shared_->ownBaseAllocator = 0;
+    shared_->ownBaseAllocator = baseAllocator ? 0 : baseAllocator_;
     shared_->ownBuffer = false;
     shared_->refcount = 1;
   }
@@ -312,6 +317,8 @@ class MemoryPoolAllocator {
     }
     Clear();
     BaseAllocator* a = shared_->ownBaseAllocator;
+    using AtomicBool = std::atomic<bool>;
+    shared_->hadOom.~AtomicBool();
     if (shared_->ownBuffer) {
       baseAllocator_->Free(shared_);
     }
@@ -371,7 +378,10 @@ class MemoryPoolAllocator {
     LOCK_GUARD;
     if (sonic_unlikely(shared_->chunkHead->size + size >
                        shared_->chunkHead->capacity)) {
-      if (!AddChunk(cp_.ChunkSize(size))) return NULL;
+      if (!AddChunk(cp_.ChunkSize(size))) {
+        shared_->hadOom.store(true, std::memory_order_release);
+        return NULL;
+      }
     }
 
     void* buffer = GetChunkBuffer(shared_) + shared_->chunkHead->size;
@@ -412,9 +422,22 @@ class MemoryPoolAllocator {
       if (originalSize) std::memcpy(newBuffer, originalPtr, originalSize);
       return newBuffer;
     }
+    // Mark OOM even on the Malloc-copy fallback so the flag is set
+    // regardless of which internal path actually failed.
+    shared_->hadOom.store(true, std::memory_order_release);
     return nullptr;
   }
 
+  // Lets callers distinguish an OOM from a logical null (e.g. Malloc(0)).
+  bool HadOom() const {
+    sonic_assert(shared_->refcount > 0);
+    return shared_->hadOom.load(std::memory_order_acquire);
+  }
+  void ClearOom() {
+    sonic_assert(shared_->refcount > 0);
+    shared_->hadOom.store(false, std::memory_order_release);
+  }
+
   //! Frees a memory block (concept Allocator)
   static void Free(void* ptr) noexcept { (void)ptr; }  // Do nothing
 
@@ -486,7 +509,8 @@ class MapAllocator {
   MapAllocator(const MapAllocator& rhs) : alloc_(rhs.alloc_) {}
 
   pointer allocate(size_type n, const void* = nullptr) {
-    return (T*)alloc_->Malloc(n * sizeof(T));
+    if (alloc_ == nullptr || n == 0) return nullptr;
+    return static_cast<pointer>(alloc_->Malloc(n * sizeof(T)));
   }
 
   void deallocate(void* p, size_type) { alloc_->Free(p); }

include/sonic/dom/dynamicnode.h

@@ -32,6 +32,9 @@
 
 namespace sonic_json {
 
+// OOM invariant: mutating operations (Reserve, AddMember, PushBack, ...)
+// leave the node unchanged on allocation failure rather than propagating an
+// error. Callers that need to detect OOM should use Allocator::HadOom().
 template <typename Allocator = SONIC_DEFAULT_ALLOCATOR>
 class DNode : public GenericNode<DNode<Allocator>> {
  public:
@@ -82,6 +85,11 @@ class DNode : public GenericNode<DNode<Allocator>> {
         this->o.len = rhs.getTypeAndLen();  // Copy size and type.
         if (count > 0) {
           void* mem = containerMalloc<MemberNode>(count, alloc);
+          if (sonic_unlikely(mem == nullptr)) {
+            this->setLength(0, kObject);
+            setChildren(nullptr);
+            break;
+          }
           rhsNodeType* rn = rhs.getObjChildrenFirst();
           DNode* ln = (DNode*)((char*)mem + sizeof(MetaNode));
           for (size_t i = 0; i < count * 2; i += 2) {
@@ -98,8 +106,13 @@ class DNode : public GenericNode<DNode<Allocator>> {
         size_t a_size = rhs.Size();
         this->a.len = rhs.getTypeAndLen();  // Copy size and type.
         if (a_size > 0) {
-          rhsNodeType* rn = rhs.getArrChildrenFirst();
           void* mem = containerMalloc<DNode>(a_size, alloc);
+          if (sonic_unlikely(mem == nullptr)) {
+            this->setLength(0, kArray);
+            setChildren(nullptr);
+            break;
+          }
+          rhsNodeType* rn = rhs.getArrChildrenFirst();
           DNode* ln = (DNode*)((char*)mem + sizeof(MetaNode));
           for (size_t i = 0; i < a_size; ++i) {
             new (ln + i) DNode(*(rn + i), alloc, copyString);
@@ -302,14 +315,17 @@ class DNode : public GenericNode<DNode<Allocator>> {
   bool CreateMap(Allocator& alloc) {
     sonic_assert(this->IsObject());
     sonic_assert(this->Capacity() >= this->Size());
-    // if (this->Size() == 0) return false;
+    // Empty object: reserve meta storage first so children() is non-null.
+    // If the reserve OOMs, children() stays null and we bail instead of
+    // dereferencing it via getMapUnsafe() / setMap.
     if (nullptr == children()) {
       this->memberReserveImpl(16, alloc);
+      if (nullptr == children()) return false;
     }
-    if (getMapUnsfe()) return true;
+    if (getMapUnsafe()) return true;
     map_type* map = static_cast<map_type*>(alloc.Malloc(sizeof(map_type)));
+    if (nullptr == map) return false;
     new (map) map_type(MAType(&alloc));
-    // SetMap(map);
     MemberNode* m = (MemberNode*)getObjChildrenFirstUnsafe();
     for (size_t i = 0; i < this->Size(); ++i) {
       map->emplace(std::make_pair((m + i)->name.GetStringView(), i));
@@ -514,8 +530,9 @@ class DNode : public GenericNode<DNode<Allocator>> {
 
   DNode& reserveImpl(size_t new_cap, Allocator& alloc) {
     if (new_cap > this->Capacity()) {
-      setChildren(containerRealloc<DNode>(children(), this->Capacity(), new_cap,
-                                          alloc));
+      void* mem =
+          containerRealloc<DNode>(children(), this->Capacity(), new_cap, alloc);
+      if (sonic_likely(mem != nullptr)) setChildren(mem);
     }
     return *this;
   }
@@ -611,10 +628,13 @@ class DNode : public GenericNode<DNode<Allocator>> {
     if (new_cap > this->Capacity()) {
       void* old_ptr = children();
       size_t old_cap = this->Capacity();
-      setChildren(
-          containerRealloc<MemberNode>(old_ptr, old_cap, new_cap, alloc));
-      if (old_cap == 0) {
-        setMap(nullptr);  // Set map as nullptr when first alloc memory.
+      void* mem =
+          containerRealloc<MemberNode>(old_ptr, old_cap, new_cap, alloc);
+      if (sonic_likely(mem != nullptr)) {
+        setChildren(mem);
+        if (old_cap == 0) {
+          setMap(nullptr);  // Set map as nullptr when first alloc memory.
+        }
       }
     }
     return *this;
@@ -650,10 +670,9 @@ class DNode : public GenericNode<DNode<Allocator>> {
   sonic_force_inline void* containerMalloc(size_t cap, Allocator& alloc) {
     size_t alloc_size = cap * sizeof(T) + sizeof(MetaNode);
     void* mem = alloc.Malloc(alloc_size);
-    // init Metanode
-    MetaNode* meta = static_cast<MetaNode*>(mem);
-    new (meta) MetaNode(cap);
-
+    if (sonic_likely(mem != nullptr)) {
+      new (static_cast<MetaNode*>(mem)) MetaNode(cap);
+    }
     return mem;
   }
 
@@ -663,10 +682,9 @@ class DNode : public GenericNode<DNode<Allocator>> {
     size_t old_size = old_cap * sizeof(T) + sizeof(MetaNode);
     size_t new_size = new_cap * sizeof(T) + sizeof(MetaNode);
     void* mem = alloc.Realloc(old_ptr, old_size, new_size);
-    // init Metanode
-    MetaNode* meta = static_cast<MetaNode*>(mem);
-    meta->SetMetaCap(new_cap);
-
+    if (sonic_likely(mem != nullptr)) {
+      static_cast<MetaNode*>(mem)->SetMetaCap(new_cap);
+    }
     return mem;
   }
 
@@ -740,7 +758,7 @@ class DNode : public GenericNode<DNode<Allocator>> {
     return ((MetaNode*)(this->o.next.children))->map;
   }
 
-  sonic_force_inline map_type* getMapUnsfe() const {
+  sonic_force_inline map_type* getMapUnsafe() const {
     sonic_assert(this->IsObject());
     return ((MetaNode*)(this->o.next.children))->map;
   }
@@ -810,20 +828,25 @@ class DNode : public GenericNode<DNode<Allocator>> {
     size_t count = this->Size();
     if (count >= this->Capacity()) {
       if (this->Capacity() == 0) {
-        setChildren(containerMalloc<MemberNode>(k_default_obj_cap, alloc));
+        void* mem = containerMalloc<MemberNode>(k_default_obj_cap, alloc);
+        if (sonic_unlikely(mem == nullptr)) return this->MemberEnd();
+        setChildren(mem);
       } else {
         size_t cap = this->Capacity();
         cap += (cap + 1) / 2;  // grow by factor 1.5
         void* old_ptr = children();
-        setChildren(containerRealloc<MemberNode>(old_ptr, this->Capacity(), cap,
-                                                 alloc));
+        void* mem =
+            containerRealloc<MemberNode>(old_ptr, this->Capacity(), cap, alloc);
+        if (sonic_unlikely(mem == nullptr)) return this->MemberEnd();
+        setChildren(mem);
       }
     }
 
     // add member to the last pos
     DNode name;
     if (copyKey) {
       name.SetString(key, alloc);
+      if (sonic_unlikely(name.IsNull())) return this->MemberEnd();
     } else {
       name.SetString(key);
     }
@@ -845,11 +868,11 @@ class DNode : public GenericNode<DNode<Allocator>> {
     if (nullptr == children()) {
       goto not_find;
     }
-    if (getMapUnsfe()) {
-      auto it = getMapUnsfe()->find(MSType(key.data(), key.size()));
-      if (it != getMapUnsfe()->end()) {
+    if (getMapUnsafe()) {
+      auto it = getMapUnsafe()->find(MSType(key.data(), key.size()));
+      if (it != getMapUnsafe()->end()) {
         m = memberBeginUnsafe() + it->second;
-        getMapUnsfe()->erase(it);
+        getMapUnsafe()->erase(it);
         goto find;
       }
 
@@ -927,9 +950,9 @@ class DNode : public GenericNode<DNode<Allocator>> {
     if (this->Size() >= cap) {
       size_t new_cap = cap ? cap + (cap + 1) / 2 : k_default_array_cap;
       void* old_ptr = this->a.next.children;
-      DNode* new_child =
-          (DNode*)containerRealloc<DNode>(old_ptr, cap, new_cap, alloc);
-      this->a.next.children = new_child;
+      void* new_ptr = containerRealloc<DNode>(old_ptr, cap, new_cap, alloc);
+      if (sonic_unlikely(new_ptr == nullptr)) return *this;
+      this->a.next.children = new_ptr;
     }
     // add value to the last pos
     DNode& last = *(this->End());