OpenClonk Bugtracker - OpenClonk
View Issue Details
0000583OpenClonkEngine - C4Scriptpublic2011-02-24 00:542017-12-23 11:37
ReporterGünther 
Assigned To 
PrioritynormalSeverityminorReproducibilityalways
StatusacknowledgedResolutionopen 
PlatformOSOS Version
Product Version 
Target Versiongit masterFixed in Version 
Summary0000583: Script Parse errors can leak C4Strings
DescriptionMakeSetter stores a reference to a C4String* in a C4AulBCC on the stack. Normally, this reference is later moved to a BCC in Code and cleaned up in ClearCode. But a script error can prevent that, and the reference is leaked.

This isn't a real problem since it's at most one tiny string per script error, and since the strings are interned even huge amount of errors over a long run of the engine won't pose a problem.

But it'd be nice to not make the C4String leak detector trigger, without risking a slowdown in the normal case.
TagsNo tags attached.
related to 0000742acknowledged  C4String leak in Boomshire.ocs 
Attached Files

Notes
(0001597)
Newton   
2011-02-24 10:52   
Hmm, I don't really understand the issue here. Will the engine crash, or be slower (memleak?), or something else? And, in what case would that happen?
(0001599)
Günther   
2011-02-24 21:50   
The only real problem is that debug engines will trigger an assertion on exit. The amount of leaked memory because of this bug is insignificant (just a few strings that would be kept in memory anyway if the script error hadn't occured), but I want to keep that assertion to catch bugs that do leak a significant amount of memory.
(0002537)
Sven2   
2013-03-17 15:29   
(Last edited: 2013-03-17 15:30)
For me (MSVC10), the assertion on shutdown triggers a bug in the custom assertion handler because some resources are already freed at that time. This crashes the engine in a debug build.

It also sucks because when I test a minimal scenario that has LocalOnly=1 definitions, there are a couple of script error in System.ocg. This means I cannot check for memory leaks with this minimal scenario.

Why not just put the C4String * into an auto_ptr?

(0002538)
Isilkor   
2013-03-17 17:05   
(Last edited: 2013-03-17 17:08)
> Why not just put the C4String * into an auto_ptr?
Please don't ever use auto_ptr. Use std::unique_ptr (C++11) or boost::scoped_ptr (C++98) instead.

> It also sucks because when I test a minimal scenario that has LocalOnly=1 definitions, there are a couple of script error in System.ocg.
IMO this is a bug in System.ocg; global functions that require specific definitions to be loaded probably shouldn't be in there.

(0005072)
occ   
2016-04-23 19:29   
Hi! There's been a check-in that references this bug. For more information you can visit the repository browser at this address:
https://git.openclonk.org/openclonk.git/commitdiff/6866375c6742d2be1bbd886306ffc69d14b8cafc

Changeset 6866375 by Günther Brammer <gbrammer@gmx.de>
Don't leak references to Strings on script errors (0000583)

(0005077)
occ   
2016-04-24 19:15   
Hi! There's been a check-in that references this bug. For more information you can visit the repository browser at this address:
https://git.openclonk.org/openclonk.git/commitdiff/e22ca51f722dbdae61fe0514de3727e36b90f4b0

Changeset e22ca51 by Günther Brammer <gbrammer@gmx.de>
Don't leak references to Strings on script errors (0000583)

(0005963)
Luchs   
2017-12-23 11:37   
Possibly fixed now? In any case, not important for this particular release.

Issue History
2011-02-24 00:54GüntherNew Issue
2011-02-24 10:52NewtonNote Added: 0001597
2011-02-24 21:50GüntherNote Added: 0001599
2013-01-08 23:19NewtonRelationship addedrelated to 0000742
2013-03-17 15:29Sven2Note Added: 0002537
2013-03-17 15:30Sven2Note Edited: 0002537bug_revision_view_page.php?bugnote_id=2537#r417
2013-03-17 17:05IsilkorNote Added: 0002538
2013-03-17 17:08IsilkorNote Edited: 0002538bug_revision_view_page.php?bugnote_id=2538#r419
2015-10-16 00:33Sven2Target Version => 7.0
2015-11-30 12:11ClonkonautStatusnew => acknowledged
2015-11-30 12:11ClonkonautTarget Version7.0 => 8.0
2016-04-23 19:29occNote Added: 0005072
2016-04-24 19:15occNote Added: 0005077
2017-12-23 11:35LuchsTarget Version8.0 => git master
2017-12-23 11:37LuchsNote Added: 0005963