Adding to TFPGList<AnsiString> doesn't increase string's refcount, which causes errors later
Original Reporter info from Mantis: Michalis @michaliskambi
-
Reporter name: Michalis Kamburelis
Original Reporter info from Mantis: Michalis @michaliskambi
- Reporter name: Michalis Kamburelis
Description:
(The title of this bug is my suspected guess, only 99% sure, about the cause of this error.)
Attached testcase creates a string list by "specialize TFPGList&LtPos;string>", then adds and removes some items. After some operations, the contents of the strings are garbage. My guess is that it's because TFPGList.Add doesn't increase the string reference count. The string pointer is directly copied by System.Move (inside TFPSList.CopyItem, called by TFPSList.Add), and there's no moment where string reference is correctly increased.
$ fpc -gl -gh ttt2.pas
$ ./ttt2
....
Messages before Add : 3
Messages[0]: 3078402488 16 You pick "Sword"
Messages[1]: 3078402824 31 You're using weapon "Sword" now
Messages[2]: 3078303848 34 Hint: press "Escape" for game menu
Messages after Add : 4
Messages[0]: 3078402488 16 You pick "Sword"
Messages[1]: 3078402824 31 You're using weapon "Sword" now
Messages[2]: 3078303848 -252645136 ... ~infinite memory garbage
First column after Messages[x] shows the AnsiString pointer, prooving it's correctly preserved. Second column after Messages[x] shows Length of string, as you can see it's a negative nonsense at the end. Without -gh, the Messages[2] may have zero length instead of garbage --- in any case, the error is still there, just more obvious with -gh.
Trivial workaround by changing TFPGList.Add parameter "const Item: T" to just "Item: T" doesn't help. Probably because it only means that string reference is incremented for the short time of TFPGList.Add, and decremented back when TFPGList.Add exits.
If my understanding of what is the cause is correct, then this may require some more serious changes to TFPGList implementation (hence priority=major). Ideas:
-
Either add TFPSList.Incref, analogous to current TFPSList.Deref, and make every relevant TFPSList operation call it. Overridden TFPGList.Deref calls currently Finalize, that decreases references. Overridden TFPGList.Incref should call something that increases references (does there exist built-in procedure for this?).
-
Or reimplement TFPGList to not be a descendant of TFPSList. Instead, TFPGList could manage internal list of type T, without resorting to inherited TFPSList helpers. This way, adding an item into the list could be implemented using a normal assignment operator (instead of System.Move, that has to be used by TFPSList). This would make reference management correct.
This issue affects also lists of records that contain AnsiString (and other ref-counted types). Although TFPGList cannot be used now for records (because TFPGList.IndexOf requires a comparison operator), see #15480 (closed), but it's possible to define your own list of records avoiding IndexOf (see my https://vrmlengine.svn.sourceforge.net/svnroot/vrmlengine/trunk/kambi_vrml_game_engine/src/base/genericstructlist.pas , see comments in #15480 (closed)). Such TGenericStructList has the same problems (when used with records with AnsiStrings), since it still descends from TFPSList.
Observed with FPC trunk (2.7.1) from 18231, and also with FPC 2.4.4.
I can probably take a shot at making a patch, for either 1 or 2 solution, if you agree with my analysis. In case of 1., I'll also need to know what should Incref actually do --- is there a "magic" procedure similar to Initialize/Finalize that increases ref count of the parameter?
Mantis conversion info:
- Mantis ID: 20005
- OS: Debian GNU/Linux
- OS Build: (testing)
- Platform: i386
- Fixed in version: 2.6.0
- Fixed in revision: 19018 (#a640bd0e)