| View Issue Details [ Jump to Notes ] | [ Issue History ] [ Print ] | ||||||||||||
| ID | Project | Category | View Status | Date Submitted | Last Update | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 0000583 | OpenClonk | Engine - C4Script | public | 2011-02-24 00:54 | 2017-12-23 11:37 | ||||||||
| Reporter | Günther | ||||||||||||
| Assigned To | |||||||||||||
| Priority | normal | Severity | minor | Reproducibility | always | ||||||||
| Status | acknowledged | Resolution | open | ||||||||||
| Product Version | |||||||||||||
| Target Version | git master | Fixed in Version | |||||||||||
| Summary | 0000583: Script Parse errors can leak C4Strings | ||||||||||||
| Description | MakeSetter 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. | ||||||||||||
| Tags | No tags attached. | ||||||||||||
| Attached Files |
| ||||||||||||
 Relationships |
||||||
|
||||||
 Relationships |
| Notes |
|
|
Newton (administrator) 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? |
|
Günther (developer) 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. |
|
Sven2 (developer) 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? |
|
Isilkor (developer) 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. |
|
occ (reporter) 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) |
|
occ (reporter) 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) |
|
Luchs (administrator) 2017-12-23 11:37 |
Possibly fixed now? In any case, not important for this particular release. |
| Notes |
| Issue History |
|||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2011-02-24 00:54 | Günther | New Issue | |
| 2011-02-24 10:52 | Newton | Note Added: 0001597 | |
| 2011-02-24 21:50 | Günther | Note Added: 0001599 | |
| 2013-01-08 23:19 | Newton | Relationship added | related to 0000742 |
| 2013-03-17 15:29 | Sven2 | Note Added: 0002537 | |
| 2013-03-17 15:30 | Sven2 | Note Edited: 0002537 | View Revisions |
| 2013-03-17 17:05 | Isilkor | Note Added: 0002538 | |
| 2013-03-17 17:08 | Isilkor | Note Edited: 0002538 | View Revisions |
| 2015-10-16 00:33 | Sven2 | Target Version | => 7.0 |
| 2015-11-30 12:11 | Clonkonaut | Status | new => acknowledged |
| 2015-11-30 12:11 | Clonkonaut | Target Version | 7.0 => 8.0 |
| 2016-04-23 19:29 | occ | Note Added: 0005072 | |
| 2016-04-24 19:15 | occ | Note Added: 0005077 | |
| 2017-12-23 11:35 | Luchs | Target Version | 8.0 => git master |
| 2017-12-23 11:37 | Luchs | Note Added: 0005963 | |
| Issue History |
