Below is for ansistring, but same should apply to dyn arrays.

Procedure fpc_AnsiStr_Incr_Ref (S : Pointer);
[Public,Alias:'FPC_ANSISTR_INCR_REF'];  compilerproc; inline;
Begin
   If S=Nil then
     exit;
// anytime after this line:
// other thread, and only ref-count holder calls fpc_ansistr_decr_ref
// and finishes before "inclock" gets the lock  => so memory gets freed
   If PAnsiRec(S-AnsiFirstOff)^.Ref<0 then exit;
   inclocked(PAnsiRec(S-AnsiFirstOff)^.Ref);
end;

what happens if another thread holds the only reference to the string (ref count = 1), and the other thread calls and finishes fpc_ansistr_decr_ref before inclocked reaches the point where it actually gets the lock?
In that case the memory holding the ref-count would be freed, before inclock acquires the lock?

----------
What I was actually thinking off was an potential optimization proposal.

If a local string var has a refcount of 1, then it can not be accessed by any other thread. Therefore it needs no lock for decreasing the ref.

- The only one ref has to be hold by the current thread
- Since it is a local var no other thread can see the identifier, and therefore this var can not be inside a fpc_AnsiStr_Incr_Ref in another thread

You could have a pointer (no ref count) to the string, but passing that to another thread would be unsafe anyway, and not get anymore unsafe by such a change.

This

Martin wrote

If a local string var has a refcount of 1, then it can not be accessed by any other thread. Therefore it needs no lock for decreasing the ref.

You would need a full memory barrier before checking whether the reference count is one, otherwise it could have been increased to two by another thread on another core quite a while ago without that change being already visible on the current core. This would probably slow down things much more than any potential gain from not performing the atomic decrement.

Jonas

Hi,

but if another core can do anything to the string, the refcount should already be 2, one for this core and one for the other core, should it not?

UniqueString has such a test:

Function fpc_ansistr_Unique(Var S : Pointer): Pointer; [Public,Alias :
'FPC_ANSISTR_UNIQUE']; compilerproc; inline;
{
   Make sure reference count of S is 1,
   using copy-on-write semantics.
}
begin
   pointer(result) := pointer(s);
   If Pointer(S)=Nil then
     exit;
   if PAnsiRec(Pointer(S)-AnsiFirstOff)^.Ref1 then
     result:=fpc_truely_ansistr_unique(s);
end;

Bye,
Benito

Benito van der Zander wrote

but if another core can do anything to the string, the refcount should already be 2, one for this core and one for the other core, should it not?

The problem is that the different cores may see different values. Core 1 could see that the refcount is 1 (wrong) while core 2 could see that is 2 (correct) unless you add memory barriers (directly, or indirectly by using atomic read-modify-write instructions).

This function is indeed not threadsafe (and it cannot be made 100% threadsafe, although it could be made a bit safer —and slower— by changing the check into an atomic inc/decrement with 0 and checking that the result is 1).

Jonas

Benito van der Zander wrote

Hi, but if another core can do anything to the string, the refcount should already be 2, one for this core and one for the other core, should it not?

No:

// global var
var
   Foo: String;
// main thread
Foo := getsomestring_with_refcount_1();
TMyThread.Create(false); // not suspended / start
Foo := ''; // de-ref
//TMyThread.Execute
LocalFoo := Foo; // copy from the global var

If the main threads "Foo:=''; "   is executed ...
- beginning right after fpc_AnsiStr_Incr_Ref in TMyThread did the NIL test
- and finished before the "inclocked"
then the inclocked operates in memory freed in the main thread?

Unless there is some thread save checking each time a global var is accessed?

This is a different issue. Managed types are indeed only threadsafe as long as there is no race condition that could cause their refcount to become zero at some point.

The issue I was talking about is the fact that atomic operations do function as global memory synchronisation operations across all cores (at least not on all architectures). If core 1 atomatically increases refcount to two and you "then" load the same refcount normally (without an atomic read-modify-exchange oepration) on another core, this other core may still see the old value.

The reason "then" is between quotes is because there is no forced ordering between these two operations (since there is no synchronisation/memory barrier linking the the two), which is also the basis of the problem.

See e.g.
https://homes.cs.washington.edu/~bornholt/post/memory-models.html for more information.

Jonas

Yes, I did drop the optimization idea after your first reply.

Hi,

Is that really so?

The ref count is stored in the same memory block as the string itself.

If core 2 could not see the new ref count, it could not see what is in the string and thus not use the string for anything .

Bye,
Benito

Am Do., 3. Jan. 2019, 13:25 hat Benito van der Zander geschrieben:

Nobody is talking about the string content. It's only about the reference count right now and that one *can* differ between cores if that isn't correctly handled (even if the string content stays the same).

Regards,
Sven

Hi,

03.01.2019 15:25, Benito van der Zander:

The ref count is stored in the same memory block as the string itself. If core 2 could not see the new ref count, it could not see what is in the string and thus not use the string for anything .

If string content modification is supposed to be possible and the said string is accessible by 2 or more threads, then some sort of explicite syncronization will need to be used (like with many other cases of data sharing across multiple threads). Pascal does not provide complete automatic thread-safety for strings (AFAIK). It does provide automatic lifecycle management (refcounting) only.

Regards,
Nikolai

Hi Fpc Developers',

Nobody is talking about the string content.

But when they are stored together, that is the same. You can't get one without the other.

When core A creates a string to pass to core B, core A writes three things: ref count 1, the string content and the incremented ref count 2.

If core B could see stale data after the second write (with ref count 1), it could also see stale data after the first write (without content). Nothing changes on core B between the writes.

It's only about the reference count right now and that one *can* differ between cores if that isn't correctly handled (even if the string content stays the same).

Sure one core can read 2 after the other core wrote 1.

I do not see how a core can read 1 after a 2 was written.

Cheers,
Benito

You can, since the data can be stored in a different cache line than the refcount. But the principle is indeed the same: the data is not guaranteed to be synchronised across cores either.

Sure one core can read 2 after the other core wrote 1. I do not see how a core can read 1 after a 2 was written.

There is no difference between the two cases as long as the second read happens without an atomic read-modify-write operation..

Jonas

Well it is getting interesting here...

At first, it is normal that 2 threads can see different data. Same applies to other types (i.e. not limited to managed types). If a thread does "SomeInt := 42", other threads may still see the previous values. That is ok, for all that the other threads know, the "some thread" may not even have reached the statement yet.

But if we look at "FreeAndNil(SomeObject)", other threads must make sure (synchronize ...), that they get the same value. Otherwise they access memory that was freed, and there is no guarantee they still see the old memory, never mind what happens on write access.

In any case for any variable that is split across several mem locations, access to the old "pointer" is dangerous, unless the programmer has taken care that this particular part can not change.

That means 2 threads, both accessing the same global string variable, is not allowed without extra protection.

The entire game only works if each thread has a separate variable referring the same string content (the refcount of which is then always

2, guaranteeing the referenced data remains allocated and in place). 

If one thread changes its data, it makes a copy, and the refcount goes to 1, and the other thread becomes sole owner)

The one thing that probably is easy to overlook, is how to give the 2nd thread access to the string to begin with. At that time the refcount may be 1, and therefore the variable for the 2nd thread must be assigned inside the 1st thread (the thread that owns the string). Otherwise the refcount could become 0, while the 2nd thread tries to get its copy.

So in that way, only the storage (and mem alloc and refcount) for managed variables are thread safe.
- The variable (holding the ref) is as little thread safe as any other var (none thread var)
- the content of a string (or dyn array) is neither thread save. s[5] can change in one thread, and not be known to the other. That is, if the refcount is 1 (which it shouldn't), or for dyn-array if the len does not change.

I am not sure, where (if at all) managed types and thread safety (in regards to refcount) is documented?
So I could not check, how well the complexity of the matter is represented.

My initial post, with the ability for the refcount to become 0 in between, could afaik only happen, if the 2 threads share the same (global) variable, and that is not thread save. (as the var could be nil in one thread, and outdated pointer in the other thread...) That closes my initial question, about the ref count potentially not being thread safe.

------------
Btw about threads seeing different cached versions of data. What happens in case of synchronize?

thread 1 calls Synchronize(DoFreeObject)

DoFreeObject runs in main thread: "FreeAndNil(Foo)"

Now we know that the main thread will correctly see Foo=nil.

But what will the thread 1 see?
Could the thread still see, the old value of Foo?

If so then how to I ever get a managed variable safely into a thread? (Once the thread was started).

Martin Frb wrote

Btw about threads seeing different cached versions of data. What happens in case of synchronize?

Synchronize uses an RtlEvent. That one uses OS primitives that include memory barriers, so the data will be properly synchronised across cores.

If so then how to I ever get a managed variable safely into a thread? (Once the thread was started).

It behaves the same as any other data.

In general: always use an RTLEvent, mutex, critical section or similar to access shared data. Those will ensure that the necessary memory barriers get inserted.

Jonas

Out of interest...
What about TRTLCriticalSection?
If a variable is shared between 2 threads, and only accessed within a TRTLCriticalSection?

Martin Frb wrote

Out of interest... What about TRTLCriticalSection? If a variable is shared between 2 threads, and only accessed within a TRTLCriticalSection?

From the mail you replied to:

***
In general: always use an RTLEvent, mutex, *critical section* or similar to access shared data. Those will ensure that the necessary memory barriers get inserted.
***

Jonas

Ups, apologies.

------------------------
Can't help it but to get back to my initial optimization idea.

Not necessarily to actually request it. Not sure if the savings would actually be that notice-able... But to see if in theory it would hold...

The idea was
- local variables of a procedure/function
- with a ref count of 1
Such a variable could in the epilogue be freed, without a "locked dec ref". (actually not limited to the epilogue, but wherever the compiler inserts such a decref)

The original answer was: it could not. The thread may not see, that another thread has inc'ed the refcount.

However, from my previous mail, it seems that another thread can only increment the refcount, if it has acquired a copy of the variable within memory barriers. Thus the first increment to refcnt=2 would be visible.

If another thread would just access any such variable (including via PAnsiString) without memory barrier (i.e., without lock) then during the other threads fpc_ansistr_incr_ref, the first thread may free the string by going to ref=0. (Or the other string risks seeing the string pointing to data, while it already is nil in reality).

That also holds, if the original thread stores a copy into a global var, since the other thread must get some lock before accessing the global var (and thus when the refcount goes up, this is visible).

So if the other thread must go through a lock of some form, to get the string, then the first thread is guaranteed to see refcnt=2. (or above)

- Other threads can then add more to the refcount, which the first thread does not see, but that does not matter, since the first thread is only interested in either 1 or greater than 1.
- Other threads can set it back to 1. The first thread may not see that, but that does not matter, because if the first thread sees > 1, it goes for the safe locked-decr.
  (If the other thread has set it back to 1, then no other thread can increase it again, without a lock.)

Other threads can NOT set the refcnt to 0. If they do, something is already seriously broken. As the epilogue of the function may then see the none-nil pointer, despite in reality it should be nil.

So if the function goes into its epilogue, and a local variable has a refcount of 1, then this local var can be freed without any call to fpc_ansistr_decr_ref (any lock).
Because no other thread can have a reference to that variable at that time.

---
This holds for local vars.
Because the function (and with that the thread for that function) in which the variable is declared is guaranteed to own the last ref-count.

For a global var, several threads could at the same time do "s := ''; ". Though without lock, that would be unsafe.
If access to the global var would be properly locked at all times (see my last big mail) then it might hold... (But I haven't really thought through all possibilities.)

Then again, it should. Because fpc_ansistr_decr_ref does not protect the variable and the var changes to nil, which is not seen by other threads. And the initial 2nd ref count would have to be gotten in a men-barrier lock too. Or one thread could free the mem, while the other increments the ref count.

Each actual variable (holding a ref to the string) is not protected / not thread save in it self. Therefore can/must be considered to be owned by one thread only.
Only the ref count is protected.

To rephrase it a bit simpler.

There are 2 ways threads can share strings (or dyn array)

1) Using the same variable.
In that case every access to the variable must be protected (synchronize, lock, ... => mem barrier in place)
If not, then if the string is relocated (size changed) or freed, it is not guaranteed that all threads see the new value.

Therefore using the same variable in several threads, without mem barrier, means that the code already has race conditions that lead to crashes.

If in this case a thread sees a refcount of 1 and wants to decrement it, the thread must be in a mem barrier/lock => no other thread has access to the variable. Save to free the string.

2) Using one (or more) variable per thread. Each variable is exclusive to that thread.
The initial copy to that variable must be taken in a way according to point 1 above. Therefore all threads are guaranteed to see a ref count of 2 or higher.

If the refcount goes back to 1, then only one thread has access to the string. And another thread can only get access via a mem barrier technique.

Threads are not guaranteed to see the refcount going back to 1.
- If they do not, the use the save locked decr. Which will ensure they see the correct value.
- If they do see the refcount at 1, then they can trust that value. (no other thread can have changed it since it went to 1)