<div dir="ltr">The reference count is only updated if the variable "A" was assigned to another variable. However, if "A" was global, it may still be changed by a second thread without changing its value. <div><div><br></div><div>program SwapTest;<br>uses<br>  Classes, SysUtils;<br><br>type<br><br>  { MyThread }<br><br>  MyThread = class(TThread)<br>  protected<br>    procedure Execute; override;<br>  end;<br><br>type<br>  PAnsiRec = ^TAnsiRec;<br>  TAnsiRec = Record<br>    CodePage    : TSystemCodePage;<br>    ElementSize : Word;<br>    {$ifdef CPU64}<br>    { align fields  }<br>    Dummy       : DWord;<br>    {$endif CPU64}<br>    Ref         : SizeInt;<br>    Len         : SizeInt;<br>    end;<br><br>var<br>  AStr: string;<br>  BStr: string;<br>  LThread: TThread;<br><br>function SRefCount(P : Pointer) : integer;<br>begin<br>  if P=Nil then<br>    Result:=0<br>  else<br>    Result:=PAnsiRec(P-SizeOf(TAnsiRec))^.Ref;<br>end;<br><br>procedure stringSwap(var a, b: string);<br>var<br>  t: string;<br>begin<br>  writeln('RefCount of A before assign to T=', SRefCount(pointer(a)));<br>  t := a;<br>  writeln('RefCount of A after assign to T=', SRefCount(pointer(a)));<br>  a := b;<br>  b := t;<br>  writeln('RefCount of A after at exit of stringSwap=', SRefCount(pointer(a)));<br>  writeln('RefCount of B after at exit of stringSwap=', SRefCount(pointer(B)));<br>end;<br><br>{ MyThread }<br><br>procedure MyThread.Execute;<br>begin<br>  AStr := '';<br>end;<br><br>begin<br>  LThread := MyThread.Create(false);<br>  writeln('Multi-threading=', IsMultiThread);<br>  AStr := IntToStr(123);<br>  UniqueString(AStr);<br>  BStr := IntToHex(123, 3);<br>  UniqueString(BStr);<br>  writeln('AStr="', AStr, '", Reference count before call=', SRefCount(pointer(AStr)));<br>  writeln('BStr="', BStr, '", Reference count before call=', SRefCount(pointer(BStr)));<br>  stringSwap(AStr, BStr);<br>  writeln('AStr="', AStr, '", Reference count after call=', SRefCount(pointer(AStr)));<br>  writeln('BStr="', BStr, '", Reference count after call=', SRefCount(pointer(BStr)));<br>  LThread.WaitFor;<br>  LThread.Free;<br>end.<br></div><div><br></div><div>The output is dependent on the the timing of the two threads, but in this case it occurred after the return of stringSwap, but you can still see the affect of the variable being cleared by the second thread. This global value could be assigned a new value, and that could also cause an issue as the reference in your simplified code could still reference the old value which may have been deleted.</div><div><br></div><div>Multi-threading=TRUE<br>AStr="123", Reference count before call=1<br>BStr="07B", Reference count before call=1<br>RefCount of A before assign to T=1<br>RefCount of A after assign to T=2<br>RefCount of A after on exit of stringSwap=1</div><div>RefCount of B after at exit of stringSwap=2<br>AStr="", Reference count after call=0<br>BStr="123", Reference count after call=1<br></div><div><br></div><div>Derek</div><div><br></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Jan 30, 2021 at 12:24 AM Benito van der Zander via fpc-pascal <<a href="mailto:fpc-pascal@lists.freepascal.org">fpc-pascal@lists.freepascal.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
  
    
  
  <div>
    <div id="gmail-m_-6916537110158040922smartTemplate4-template">Hi,<br>
      <p><br>
        </p><blockquote type="cite">Should "A" have a reference count of 1,
          and it is assigned a new value on another thread, its
          reference count will decrease to zero and the string and its
          descriptor are both freed. The value of "A" will be set to
          nil. </blockquote>
      <p></p>
      <p><br>
        If the reference count is 1, there is no other thread involved.
        <br>
      </p>
      If there was another thread, there would be a reference in this
      thread and in the other thread, so the reference count would be at
      least 2</div>
    <div><br>
    </div>
    <div><br>
    </div>
    <div>It is exactly as thread safe as the unoptimized function. If
      the reference count was 1, the unoptimized function would not work
      either, since the thread swap might occur just before the first
      instruction, and the strings are freed before the function can do
      anyhting<br>
    </div>
    <div><br>
    </div>
    <div><br>
      Bye,<br>
      Benito </div>
    <div>On 28.01.21 08:08, Derek Edson via
      fpc-pascal wrote:<br>
    </div>
    <blockquote type="cite">
      <div dir="ltr">Your simplified code would not be thread safe. A
        thread swap after the first instruction of<br>
         <br>
                mov (%rdi), %rax<br>
        <br>
        could potentially cause a problem. RAX (A) contains the pointer
        to the string descriptor, which includes the pointer to the
        actual string data and the reference count.<br>
        <br>
        Should "A" have a reference count of 1, and it is assigned a new
        value on another thread, its reference count will decrease to
        zero and the string and its descriptor are both freed. The value
        of "A" will be set to nil. <br>
        <br>
        RAX will not have been updated, so after<br>
        <br>
                mov %rax, (%rsi)<br>
        <br>
        "B" will be pointing to an invalid location. Its content is now
        dependent on memory reallocation. <br>
        <br>
        In this situation, changing "B" after the return from stringSwap
        procedure might again cause the string to be freed (assuming the
        memory was not reallocated), causing a double free error.<br>
        <div><br>
        </div>
        <div>Use of the internal procedures to increment the string
          reference count of both strings before your code and then
          decrementing the reference counts afterwards would probably
          work.</div>
        <div><br>
        </div>
        <div>The same probably applies to other reference count objects.</div>
        <div><br>
        </div>
        <div>Derek</div>
      </div>
      <br>
      <div class="gmail_quote">
        <div dir="ltr" class="gmail_attr">On Thu, Jan 28, 2021 at 11:35
          AM Benito van der Zander via fpc-pascal <<a href="mailto:fpc-pascal@lists.freepascal.org" target="_blank">fpc-pascal@lists.freepascal.org</a>>
          wrote:<br>
        </div>
        <blockquote class="gmail_quote">
          <div>
            <div id="gmail-m_-6916537110158040922gmail-m_3515440808062385958smartTemplate4-template">Hi,<br>
              <br>
              <p> </p>
              <blockquote type="cite">
                <div><br>
                </div>
                <div>
                  <blockquote type="cite"> Procedure
                    ManagedMove<T>(const source: T;var dest:
                    T;count: SizeInt);</blockquote>
                </div>
                <br>
                <div dir="auto">In principle a good idea. However this
                  is one of those cases where you'd definitely need to
                  use constref instead of const.</div>
              </blockquote>
              <p><br>
              </p>
              <p>Or var, since the source might be cleared<br>
              </p>
              <p> </p>
              <blockquote type="cite">
                <div dir="auto">
                  <div class="gmail_quote">
                    <blockquote class="gmail_quote">
                      <div>
                        <div>
                          <p>And perhaps there could be a special
                            attribute to mark which kind of moving is
                            needed, e.g..<br>
                          </p>
                            type [moveable] TA = record<br>
                          <div>
                            <div> </div>
                          </div>
                            type [referencecounted] TA = record</div>
                        <div>   type [nonmoveable] TA = record<br>
                        </div>
                      </div>
                    </blockquote>
                  </div>
                </div>
                <div dir="auto"><br>
                </div>
                <div dir="auto">No, thank you.</div>
                <div dir="auto"><br>
                </div>
              </blockquote>
               
              <p>But it could help a lot with optimizations. For
                moveable types it could omit calling an assignment
                function and just do a memory copy; and for refcounted
                tyes it could that when the number of references does
                not change. <br>
              </p>
              <p><br>
              </p>
              <p><br>
              </p>
              <p>Like a simple swap function:</p>
              <p>procedure stringSwap(var a, b: string);<br>
                var t: string;<br>
                begin<br>
                  t := a;<br>
                  a := b;<br>
                  b := t;<br>
                end;<br>
              </p>
              <p>A smart compiler could detect that it is a refcounted
                type and the number of references stays the same, and
                thus optimize it into:</p>
              <p>    mov    (%rdi),%rax<br>
                    mov    (%rsi),%rdx<br>
                    mov    %rdx,(%rdi)<br>
                    mov    %rax,(%rsi)<br>
                    retq   <br>
                <br>
              </p>
              <p>But FPC turns it into:</p>
              <p>    begin<br>
                    push   %rbx<br>
                    push   %r12<br>
                    lea    -0x68(%rsp),%rsp<br>
                    mov    %rdi,%rbx<br>
                    mov    %rsi,%r12<br>
                    movq   $0x0,(%rsp)<br>
                    lea    0x8(%rsp),%rdx<br>
                    lea    0x20(%rsp),%rsi<br>
                    mov    $0x1,%edi<br>
                    callq  0x4324c0 <fpc_pushexceptaddr><br>
                    mov    %rax,%rdi<br>
                    callq  0x41fba0 <fpc_setjmp><br>
                    movslq %eax,%rdx<br>
                    mov    %rdx,0x60(%rsp)<br>
                    test   %eax,%eax<br>
                    jne    0x469191 <STRINGSWAP+97><br>
                    t := a;<br>
                    mov    (%rbx),%rsi<br>
                    mov    %rsp,%rdi<br>
                    callq  0x428d00 <fpc_ansistr_assign><br>
                    a := b;<br>
                    mov    (%r12),%rsi<br>
                    mov    %rbx,%rdi<br>
                    callq  0x428d00 <fpc_ansistr_assign><br>
                    b := t;<br>
                    mov    (%rsp),%rsi<br>
                    mov    %r12,%rdi<br>
                    callq  0x428d00 <fpc_ansistr_assign><br>
                    callq  0x432830 <fpc_popaddrstack><br>
                    end;<br>
                    mov    %rsp,%rdi<br>
                    callq  0x428ca0 <fpc_ansistr_decr_ref><br>
                    mov    0x60(%rsp),%rax<br>
                    test   %rax,%rax<br>
                    je     0x4691b8 <STRINGSWAP+136><br>
                    callq  0x4329e0 <fpc_reraise><br>
                    movq   $0x0,0x60(%rsp)<br>
                    jmp    0x469191 <STRINGSWAP+97><br>
                    lea    0x68(%rsp),%rsp<br>
                    pop    %r12<br>
                    pop    %rbx<br>
                    retq   <br>
                <br>
              </p>
              <p><br>
              </p>
              <blockquote type="cite"><br>
                Then you want to simply use ismanagedtype and move(). 
                Moving the move() to a separate generic procedure will
                only lead to many instantiations of what is basically a
                move() procedure. <br>
              </blockquote>
            </div>
            <div><br>
            </div>
            <div>The goal is to reduce instantiations. <br>
            </div>
            <div>If there are n generic (collection) classes, C1, C2,
              .., Cn, and k types used in the collections, T1, T2, ..
              Tk, there are n*k collection class instantiation
              C1<T1>, C1<T2>, .. C1<Tk>, C2<T1>,
              ... Cn<Tk></div>
            <div>If each collection class does its own
              Finalize-Loop/Move/Fillchar there are also n*k of these
              move implementations. So like 3*n*k calls.<br>
            </div>
            <div>When any collection can call the same ManagedMove,
              there should only be k ManagedMove instantiations and n*k
              calls to ManagedMove, which is only one call. So it is k +
              n*k calls,  which is around a third of 3*n*k<br>
            </div>
            <div> <br>
              Bye,<br>
              Benito </div>
            <div>On 11.01.21 18:51, Sven Barth via fpc-pascal wrote:<br>
            </div>
            <blockquote type="cite">
              <div dir="auto">
                <div>
                  <div class="gmail_quote">
                    <div dir="ltr" class="gmail_attr">Benito van der
                      Zander via fpc-pascal <<a href="mailto:fpc-pascal@lists.freepascal.org" target="_blank">fpc-pascal@lists.freepascal.org</a>>
                      schrieb am Mo., 11. Jan. 2021, 15:26:<br>
                    </div>
                    <blockquote class="gmail_quote">
                      <div>
                        <div id="gmail-m_-6916537110158040922gmail-m_3515440808062385958m_926231055581021927smartTemplate4-template">Hi,</div>
                        <div><br>
                        </div>
                        <div>perhaps a  safe, generic function for this
                          copying could be added to the RTL. Like:<br>
                        </div>
                        <div><br>
                        </div>
                        <div> Procedure ManagedMove<T>(const
                          source: T;var dest: T;count: SizeInt);</div>
                      </div>
                    </blockquote>
                  </div>
                </div>
                <div dir="auto"><br>
                </div>
                <div dir="auto">In principle a good idea. However this
                  is one of those cases where you'd definitely need to
                  use constref instead of const.</div>
                <div dir="auto"><br>
                </div>
                <div dir="auto">
                  <div class="gmail_quote">
                    <blockquote class="gmail_quote">
                      <div>
                        <div>And when you use IsManagedType, it does not
                          distinguish standard strings with such weird
                          managed types.<br>
                        </div>
                      </div>
                    </blockquote>
                  </div>
                </div>
                <div dir="auto"><br>
                </div>
                <div dir="auto">You can additionally use GetTypeKind as
                  well. Unlike TypeInfo it directly returns the
                  TTypeKind (which for this case is enough) and is
                  considered constant. </div>
                <div dir="auto"><br>
                </div>
                <div dir="auto">
                  <div class="gmail_quote">
                    <blockquote class="gmail_quote">
                      <div>
                        <div>
                          <p>And perhaps there could be a special
                            attribute to mark which kind of moving is
                            needed, e.g..<br>
                          </p>
                            type [moveable] TA = record<br>
                          <div>
                            <div> </div>
                          </div>
                            type [referencecounted] TA = record</div>
                        <div>   type [nonmoveable] TA = record<br>
                        </div>
                      </div>
                    </blockquote>
                  </div>
                </div>
                <div dir="auto"><br>
                </div>
                <div dir="auto">No, thank you.</div>
                <div dir="auto"><br>
                </div>
                <div dir="auto">Regards, </div>
                <div dir="auto">Sven </div>
              </div>
              <br>
              <fieldset></fieldset>
              <pre>_______________________________________________
fpc-pascal maillist  -  <a href="mailto:fpc-pascal@lists.freepascal.org" target="_blank">fpc-pascal@lists.freepascal.org</a>
<a href="https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal" target="_blank">https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal</a>
</pre>
            </blockquote>
          </div>
          _______________________________________________<br>
          fpc-pascal maillist  -  <a href="mailto:fpc-pascal@lists.freepascal.org" target="_blank">fpc-pascal@lists.freepascal.org</a><br>
          <a href="https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal" rel="noreferrer" target="_blank">https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal</a><br>
        </blockquote>
      </div>
      <br>
      <div><br>
      </div>
      -- <br>
      <div dir="ltr">Derek Edson</div>
      <br>
      <fieldset></fieldset>
      <pre>_______________________________________________
fpc-pascal maillist  -  <a href="mailto:fpc-pascal@lists.freepascal.org" target="_blank">fpc-pascal@lists.freepascal.org</a>
<a href="https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal" target="_blank">https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal</a>
</pre>
    </blockquote>
  </div>

_______________________________________________<br>
fpc-pascal maillist  -  <a href="mailto:fpc-pascal@lists.freepascal.org" target="_blank">fpc-pascal@lists.freepascal.org</a><br>
<a href="https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal" rel="noreferrer" target="_blank">https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature">Derek Edson</div>