diff --git a/vlib/toml/any.v b/vlib/toml/any.v index 1d28abfa58..93307d8db3 100644 --- a/vlib/toml/any.v +++ b/vlib/toml/any.v @@ -21,7 +21,11 @@ pub type Any = Null // string returns `Any` as a string. pub fn (a Any) string() string { match a { - string { return a as string } + // NOTE if `.clone()` is not used here: + // string { return a as string } + // ... certain call-patterns to this function will cause a memory corruption. + // See `tests/toml_memory_corruption_test.v` for a matching regression test. + string { return (a as string).clone() } time.Time { return a.format_ss_micro() } else { return a.str() } } diff --git a/vlib/toml/tests/compact_test.v b/vlib/toml/tests/compact_test.v index a5267280a6..c9ebc5b7f1 100644 --- a/vlib/toml/tests/compact_test.v +++ b/vlib/toml/tests/compact_test.v @@ -35,18 +35,13 @@ fn test_parse_compact_text() { assert title == toml.Any('TOML Example') assert title as string == 'TOML Example' - owner := toml_doc.value('owner') as map[string]toml.Any - any_name := owner.value('name') or { panic(err) } - assert any_name.string() == 'Tom Preston-Werner' - database := toml_doc.value('database') as map[string]toml.Any db_serv := database['server'] or { panic('could not access "server" index in "database" variable') } assert db_serv as string == '192.168.1.1' - // TODO BUG depending on WHAT directory the tests is run from, this one assert sometimes fail?!?! - // assert toml_doc.value('owner.name') as string == 'Tom Preston-Werner' + assert toml_doc.value('owner.name') as string == 'Tom Preston-Werner' assert toml_doc.value('database.server') as string == '192.168.1.1' diff --git a/vlib/toml/tests/toml_memory_corruption_test.v b/vlib/toml/tests/toml_memory_corruption_test.v new file mode 100644 index 0000000000..d5cde5b829 --- /dev/null +++ b/vlib/toml/tests/toml_memory_corruption_test.v @@ -0,0 +1,24 @@ +// This tests the `toml` module for a known memory corruption. +// The BUG shows below if no string `.clone()` nor any garbage-collection is done... +import os +import toml + +const toml_text = os.read_file(os.real_path(os.join_path(os.dir(@FILE), 'testdata', 'toml_test')) + + '.toml') or { panic(err) } + +fn test_toml_known_memory_corruption() { + toml_doc := toml.parse(toml_text) or { panic(err) } + + owner := toml_doc.value('owner') as map[string]toml.Any + any_name := owner.value('name') or { panic(err) } + // This assert code path will cause the corruption. + assert any_name.string() == 'Tom Preston-Werner' + + // This code then triggered the bug before the fix. + // Also see note in toml/any.v in function `pub fn (a Any) string() string` + assert toml_doc.value('owner.name') as string == 'Tom Preston-Werner' + + // Repeat the pattern + assert any_name.string() == 'Tom Preston-Werner' + assert toml_doc.value('owner.name') as string == 'Tom Preston-Werner' +} diff --git a/vlib/toml/tests/toml_test.v b/vlib/toml/tests/toml_test.v index fb1ebe4786..f68674ddad 100644 --- a/vlib/toml/tests/toml_test.v +++ b/vlib/toml/tests/toml_test.v @@ -24,18 +24,13 @@ fn test_toml() { assert title == toml.Any('TOML Example') assert title as string == 'TOML Example' - owner := toml_doc.value('owner') as map[string]toml.Any - any_name := owner.value('name') or { panic(err) } - assert any_name.string() == 'Tom Preston-Werner' - database := toml_doc.value('database') as map[string]toml.Any db_serv := database['server'] or { panic('could not access "server" index in "database" variable') } assert db_serv as string == '192.168.1.1' - // TODO BUG depending on WHAT directory the tests is run from, this one assert sometimes fail?!?! - // assert toml_doc.value('owner.name') as string == 'Tom Preston-Werner' + assert toml_doc.value('owner.name') as string == 'Tom Preston-Werner' assert toml_doc.value('database.server') as string == '192.168.1.1' diff --git a/vlib/toml/toml.v b/vlib/toml/toml.v index c644a79508..ba102a465d 100644 --- a/vlib/toml/toml.v +++ b/vlib/toml/toml.v @@ -92,7 +92,6 @@ pub fn (d Doc) to_json() string { // value queries a value from the TOML document. pub fn (d Doc) value(key string) Any { values := d.ast.table as map[string]ast.Value - // any_values := d.ast_to_any(values) as map[string]Any return d.get_map_value_as_any(values, key) }