Use GC.preserve around string parsing#395
Conversation
9eb961f to
501cc88
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #395 +/- ##
==========================================
+ Coverage 89.89% 89.99% +0.09%
==========================================
Files 7 7
Lines 1316 1329 +13
==========================================
+ Hits 1183 1196 +13
Misses 133 133 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| elseif T == JSONTypes.STRING | ||
| str, _ = parsestring(x) | ||
| Base.print(io, "JSON.LazyValue(", repr(convert(String, str)), ")") | ||
| buf = getbuf(x) |
There was a problem hiding this comment.
Since buf isn't explicitly used and getbuf is just an accessor, GC.@preserve x is probably more clear in this case. The main difference is that the other fields of x would also be preserved.
There was a problem hiding this comment.
Ah, so GC.@preserve x will ensure the field is also preserved?
There was a problem hiding this comment.
It will assume the whole object is being used in some unknown way, which includes the field.
For mutable object, this implies the object address may be used (though there’s no requirement for that address to be allocated in the heap). For immutable object this only implies that the fields are all being used in some unknown way.
Full implementation of the idea/discussion in #391 . This ensures our
bufis GC-preserved before every call toparsestringand the preservation lasts until thePtrStringgoes out of scope.Many thanks to @yuyichao for helping around this issue.