The Delphi Bug List

Entry No.
654
RTL - Sys - System
In multithreaded applications, the UniqueString procedure in the System unit could perform an invalid memory access or leak memory.
1.02 2.01 3.0 3.01 3.02 4.0 4.01 4.02 4.03 5.0 5.01 6.0 6.01 6.02 Kylix 1.0
N/AN/AN/AN/AN/AN/AN/AN/AN/AExistsExistsFixedFixedFixedExists
Description
Reported by Steve Gardner
In multithreaded applications, the UniqueString procedure in the System unit could perform an invalid memory access or leak memory.

This could lead to rarely occurring bugs in multithreaded applications.

Justification:

This is not a reproducible bug. The bug is observed from the assembler code in System.pas.

UniqueString does the following:

  1. Exits if the string is nil, or the reference count equals 1
  2. Creates a new destination string
  3. Uses "LOCK DEC" to decrement the reference count of the source in a thread-safe way.
  4. Copies the string contents from source to destination.

Consider the scenario where there are two string variables, A and B, that refer to the same copy of a string whose reference count is 2, and one thread calls UniqueString(A) as another thread is executing "B := C". As A and B are separate variables, this should be an acceptable thing to do.

After step 3 reduces the reference count to 1, but before step 4 completes, the "B := C" thread could interrupt and release the contents of the source string. When step 4 resumes, it will be reading from released memory.

Alternatively, the "B := C" thread could interrupt step 2 and decrease the reference count to 1. When step 3 executes, it will (correctly) reduce the reference count to 0, but it will not release the associated memory.

Note from Jordan Russell:
I marked this bug as "N/A" on Delphi versions prior to 5 because those versions do not have any thread-safe string code.

Delphi 6 & Kylix:
The suggested workaround is almost exactly what was implemented in Delphi 6. I haven't tested, but I assume this means the problem must be solved.

Kylix does not have the FreeMem call, so the memory leak problem potentially still exists there.

Solution / workaround
Workaround:

Whenever a copy of a string is made for use by another thread, call UniqueString immediately after the assignment.

Bug fix:

This would require a recompilation of System.pas.

WARNING: I have not had the time to test it. Please could someone volunteer?

The process should be:

  1. Exit if the string is nil, or the reference count equals 1
  2. Create a new destination string
  3. Copy the string contents from source to destination
  4. Use "LOCK DEC" and call _FreeMem if the reference count was reduced to 0

Apply the marked changes:

// ! WARNING -- these changes are untested
procedure       UniqueString(var str: string);
asm
        { ->    EAX pointer to str              }
        { <-    EAX pointer to unique copy      }
        MOV     EDX,[EAX]
        TEST    EDX,EDX
        JE      @@exit
        MOV     ECX,[EDX-skew].StrRec.refCnt
        DEC     ECX
        JE      @@exit

        PUSH    EBX
        MOV     EBX,EAX
        MOV     EAX,[EDX-skew].StrRec.length
        CALL    _NewAnsiString
        MOV     EDX,EAX
        MOV     EAX,[EBX]
        MOV     [EBX],EDX
{
        MOV     ECX,[EAX-skew].StrRec.refCnt // ! Removed lines->
        DEC     ECX                          // !
        JL      @@skip                       // !
   LOCK DEC     [EAX-skew].StrRec.refCnt     // ! <-Removed lines
@@skip:
}
        MOV     ECX,[EAX-skew].StrRec.length
        PUSH    EAX                          // ! New line
        CALL    Move
        POP     EAX                          // ! New lines->
        MOV     ECX,[EAX-skew].StrRec.refCnt // !
        DEC     ECX                          // !
        JL      @@skip                       // !
   LOCK DEC     [EAX-skew].StrRec.refCnt     // !
        JNE     @@skip                       // !
        LEA     EAX,[EAX-skew].StrRec.refCnt // !
        CALL    _FreeMem                     // !
@@skip:                                      // ! <-New lines
        MOV     EDX,[EBX]
        POP     EBX
@@exit:
        MOV     EAX,EDX
end;
User-contributed comments
Cantacuzino Alecu Mihai
01 May 2001  12:41 PM GMT
Linked to this bug in D5.01 I am experiencing another problem with strings: Somehow I am not able to free or to use an allocated (StrAlloc) string. Sometimes it arrives that the my allocated memory space seems to be incoherent with the others blocks of allocated space, in the _FreeMem procedure. In my context I am building some DataSet compounds to connect an use a Sybase 11.0.xx Server with the DB-Lib (API). My strategy is to store records into a TList of PChars and use it as a DataSet. The bug (or whatever it is) "shown his thees" when i try to define Fields or FieldDefs and is not very stable. Results are:
- Shut down of the application, without error message, in standalone execution mode
- Destruction of the Delphi environnement (Sometimes closed, sometimes blocked or sometimes showing internal errors like U704) when debugging application.
- Same when debugging compounds.
Now this is my algorithm:
1) Send the command to the server
2-1) if Succeed then Intialize FieldDefs - Errors were founded here
2-2) Othewise throw an Exception
3) if Fields are not defined then Define Fieds - Errors were founded here
4) BindFields
5) CollectRows
6) Close Connection
7) position to the first row

When I use a PChar I do this (Allocated:PChar; i,Size:Integer):
1) Allocated:=StrAlloc(Size)
2) Fill with #0 to the Size in a personal for statement like:
for i:=0 to Size-1 do
PByte(Allocated+i)^:=0
3) Force the Size byte to 0 PByte(Allocated+Size-1)^:=0;
4...n-3) Use the Allocated - Sometimes Fails
n-2) Same as 2
n-1) StrDispose(Allocated) - Sometimes Fails
n) Allocated:=nil;
I will appreciate every helpfull solution to this.
Latest update of this entry: 2001-06-19

Post a comment on this bug


Index page
Delphi Bug List home page
The Delphi Bug Lists are presently maintained by Jordan Russell, who has taken over this task from Reinier Sterkenburg since August 2000.
All feedback is appreciated. See also the feedback section of the Delphi Bug List home page.