From 6cb6113c57a17781601736c0f87141fb40ba17c2 Mon Sep 17 00:00:00 2001 From: Christoffer Lerno Date: Sat, 31 Aug 2024 03:35:39 +0200 Subject: [PATCH] - Memory leak in Object when not using temp allocators. - Tracking allocator would double the allocations in the report. --- lib/std/collections/object.c3 | 5 +- lib/std/core/allocators/tracking_allocator.c3 | 1 - lib/std/encoding/json.c3 | 56 ++++++++++--------- releasenotes.md | 2 + src/compiler/compiler_internal.h | 2 +- 5 files changed, 35 insertions(+), 31 deletions(-) diff --git a/lib/std/collections/object.c3 b/lib/std/collections/object.c3 index dc41e416e..141d610a1 100644 --- a/lib/std/collections/object.c3 +++ b/lib/std/collections/object.c3 @@ -128,9 +128,9 @@ fn void Object.free(&self) self.array.free(); case ObjectInternalMap: self.map.@each_entry(; ObjectInternalMapEntry* entry) { - allocator::free(self.allocator, entry.key); entry.value.free(); }; + self.map.free(); default: break; } @@ -181,10 +181,9 @@ fn void Object.set_object(&self, String key, Object* new_object) @private ObjectInternalMapEntry*! entry = self.map.get_entry(key); defer { - (void)allocator::free(self.allocator, entry.key); (void)entry.value.free(); } - self.map.set(key.copy(self.map.allocator), new_object); + self.map.set(key, new_object); } macro Object* Object.object_from_value(&self, value) @private diff --git a/lib/std/core/allocators/tracking_allocator.c3 b/lib/std/core/allocators/tracking_allocator.c3 index 46148403b..ae2ae68da 100644 --- a/lib/std/core/allocators/tracking_allocator.c3 +++ b/lib/std/core/allocators/tracking_allocator.c3 @@ -87,7 +87,6 @@ fn void*! TrackingAllocator.acquire(&self, usz size, AllocInitType init_type, us backtrace::capture_current(&bt); self.map.set((uptr)data, { data, size, bt }); self.mem_total += size; - self.allocs_total++; return data; } diff --git a/lib/std/encoding/json.c3 b/lib/std/encoding/json.c3 index b6e6bbf83..98448e49a 100644 --- a/lib/std/encoding/json.c3 +++ b/lib/std/encoding/json.c3 @@ -27,11 +27,13 @@ fn Object*! temp_parse_string(String s) fn Object*! parse(InStream s, Allocator allocator = allocator::heap()) { - JsonContext context = { .last_string = dstring::new_with_capacity(64, allocator), .stream = s, .allocator = allocator }; - defer context.last_string.free(); - @pool(allocator) + @stack_mem(512; Allocator mem) { - return parse_any(&context); + JsonContext context = { .last_string = dstring::new_with_capacity(64, mem), .stream = s, .allocator = allocator }; + @pool(allocator) + { + return parse_any(&context); + }; }; } @@ -102,9 +104,9 @@ fn Object*! parse_any(JsonContext* context) @local fn JsonTokenType! lex_number(JsonContext *context, char c) @local { - @pool() + @stack_mem(256; Allocator mem) { - DString t = dstring::temp_with_capacity(32); + DString t = dstring::new_with_capacity(32, .allocator = mem); bool negate = c == '-'; if (negate) { @@ -152,32 +154,34 @@ fn JsonTokenType! lex_number(JsonContext *context, char c) @local fn Object*! parse_map(JsonContext* context) @local { Object* map = object::new_obj(context.allocator); - JsonTokenType token = advance(context)!; defer catch map.free(); + JsonTokenType token = advance(context)!; - DString temp_key = dstring::new_with_capacity(32, context.allocator); - defer temp_key.free(); - while (token != JsonTokenType.RBRACE) + @stack_mem(256; Allocator mem) { - if (token != JsonTokenType.STRING) return JsonParsingError.UNEXPECTED_CHARACTER?; - DString string = context.last_string; - if (map.has_key(string.str_view())) return JsonParsingError.DUPLICATE_MEMBERS?; - // Copy the key to our temp holder. We do this to work around the issue - // if the temp allocator should be used as the default allocator. - temp_key.clear(); - temp_key.append(string); - parse_expected(context, COLON)!; - Object* element = parse_any(context)!; - map.set(temp_key.str_view(), element); - token = advance(context)!; - if (token == JsonTokenType.COMMA) + DString temp_key = dstring::new_with_capacity(32, mem); + while (token != JsonTokenType.RBRACE) { + if (token != JsonTokenType.STRING) return JsonParsingError.UNEXPECTED_CHARACTER?; + DString string = context.last_string; + if (map.has_key(string.str_view())) return JsonParsingError.DUPLICATE_MEMBERS?; + // Copy the key to our temp holder, since our + // last_string may be used in parse_any + temp_key.clear(); + temp_key.append(string); + parse_expected(context, COLON)!; + Object* element = parse_any(context)!; + map.set(temp_key.str_view(), element); token = advance(context)!; - continue; + if (token == JsonTokenType.COMMA) + { + token = advance(context)!; + continue; + } + if (token != JsonTokenType.RBRACE) return JsonParsingError.UNEXPECTED_CHARACTER?; } - if (token != JsonTokenType.RBRACE) return JsonParsingError.UNEXPECTED_CHARACTER?; - } - return map; + return map; + }; } fn Object*! parse_array(JsonContext* context) @local diff --git a/releasenotes.md b/releasenotes.md index e70075095..8c56b7360 100644 --- a/releasenotes.md +++ b/releasenotes.md @@ -95,6 +95,8 @@ - Fix WASM memory init priority. - Fix bug with `defer (catch err)` when used together with regular defer. - Methods can now properly be aliased using `def` #1393. +- Memory leak in Object when not using temp allocators. +- Tracking allocator would double the allocations in the report. ### Stdlib changes diff --git a/src/compiler/compiler_internal.h b/src/compiler/compiler_internal.h index 81868d1f7..c836cb35d 100644 --- a/src/compiler/compiler_internal.h +++ b/src/compiler/compiler_internal.h @@ -3063,7 +3063,7 @@ INLINE bool decl_is_user_defined_type(Decl *decl) { DeclKind kind = decl->decl_kind; return (kind == DECL_UNION) | (kind == DECL_STRUCT) | (kind == DECL_BITSTRUCT) - | (kind == DECL_ENUM) | (kind == DECL_DISTINCT); + | (kind == DECL_ENUM) | (kind == DECL_TYPEDEF) || (kind == DECL_DISTINCT); } INLINE Decl *decl_flatten(Decl *decl)