|
I meant different - hard to explain.
With my existing getter, if I store reference in TMP.
And my Setter (which is locked) modifies list in another function,
tmp will still have old value?
If instead of tmp I use getter each time, I will be able to
see the changed value?
PS I am thinking to go with solution of returning copy, that seems better
|
|
|
|
|
That's where it gets complicated...
The value in tmp will only change when you use an assignment operator.
But the object it refers to can be changed!
Let's try an example.
int[] arr = new int[] {1, 2, 3, 4, 5};
int[] tmp = arr;
Both tmp and arr refer to the same array of 5 integers:
private void PrintThem(int[] data)
{
foreach (int i in data)
{
Console.WriteLine(i);
}
}
...
int[] arr = new int[] {1, 2, 3, 4, 5};
int[] tmp = arr;
PrintThem(arr);
PrintThem(tmp);
will produce the same numbers.
And if I do this:
int[] arr = new int[] {1, 2, 3, 4, 5};
int[] tmp = arr;
arr[2] = 666;
PrintThem(arr);
PrintThem(tmp);
You will get the same sequence printed twice:
1
2
666
4
5
1
2
666
4
5
It's only when I create a new array that it changes:
int[] arr = new int[] {1, 2, 3, 4, 5};
int[] tmp = arr;
arr[2] = 666;
arr = new int {7, 8, 9};
PrintThem(arr);
PrintThem(tmp);
Will produce:
7
8
9
1
2
666
4
5 If your setter modifies m_HostAddresses by assigning it a new collection:
m_HostAddresses = new List<SocketStructure>(); then there is no threading problem because the collections are different.
But if it modifies the content instead:
lock (m_locker)
{
m_HostAddresses.Add(new SocketStructure());
}
Then that isn't thread safe unless every time you access the collection you add the lock round that code as well.
Even returning a copy may not cure this problem, if any of the SocketStructure elements in the collection are altered at any point, because the copy of the collection does not generate a copy of all the elements in it, just the references to them!
This can take a bit of getting your head round, and without being able to see when your eyes start to glaze over it's very difficult to know if you understand what I mean.
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
|
|
|
|
|
I see
Everytime I modify m_HostAddresses - there is lock inside that function
Maybe I need something like this?
public static SocketStructure[] GetHostAddressesList() {
lock(m_locker)
{
List<SocketStructure>tmp = new List<SocketStructure>tmp();
for(int i = 0; i<m_HostAddress.Count(); i++)
{
SocketStructure s = new SocketStructure();
s.IP = m_HostAddress[i].IP;
s.port = m_HostAddress[i].port;
tmp.Add(s);
}
return tmp.ToList();
}
}
|
|
|
|
|
That's still "complicated" - because you may need the lock because of the m_HostAddress collection!
The List now doesn't need it because it can't be accessed outside the method: it's created inside the method, and the only reference to it - tmp - goes out of scope at the end. So technically, if you didn't have the Host Address collection involved, you wouldn't need a lock at all.
And your method doesn't have to return a copy at all, because the collection it returns is only referenced in the one place, and contains entirely new objects. It is a new object that is returned each time.
And to answer your other question: "IP is string type - but am I copying reference here again????" Oh yes. Strings are Reference types (but slightly special in that they are immutable) so every time you assign a string value you assign a reference to the actual string instance.
port is an integer - which is a Value type - so every time you assign an integer you are copying the value.
If you weren't, then things woudl get nasty quickly:
int a = 6;
int b = a;
b = b * 2; If integers were reference types then a and b would both contain 12!
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
|
|
|
|
|
confused 
Let's say this is my getter
public static SocketStructure[] GetHostAddressesList() {
lock(m_locker)
{
List<SocketStructure>tmp = new List<SocketStructure>tmp();
for(int i = 0; i<m_HostAddress.Count(); i++)
{
SocketStructure s = new SocketStructure();
s.IP = m_HostAddress[i].IP;
s.port = m_HostAddress[i].port;
tmp.Add(s);
}
return tmp.ToList();
}
}
And say the code that can add elements to my m_HostAddresses is inside ONE and only one function which inside locks on the same object (of course adding code is inside lock).
Am I fine given such scenario?
|
|
|
|
|
Yes, you should be - but you still don't need to return a copy of the collection becuase it's created anew each time the getter is called.
return tmp; will do fine.
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
|
|
|
|
|
If you want to protect your underlying list from being modified, then you need to return a copy of the list rather than the list itself. If you change your GetHostAddressesList slightly, you'll find that you are achieving a more effective lock:
public static SocketStructure[] GetHostAddressesList()
{
lock (m_locker)
{
if (!m_isInited) return null;
return m_HostAddresses.ToArray();
}
}
|
|
|
|
|
This is complicated in many many levels 
First off, if you want a read-only getter, and the list you return is small, then yeah, a copy would partially work:
public static List<SocketStructure> GetHostAddressesList() {
lock(m_locker) return m_HostAddress.ToList();
}
However, if SocketStructure is a class, you're still not completely safe.
Because you will have a copy of the list, but the elements themselves still point to your internal list - and they can still change as you access them.
What you want to return is a copy of the list (as shown above), and:
1. either have SocketStructure immutable
2. or make SocketStructure thread-safe as well.
3. make SocketStructure a struct (in that case each element in the returned list will be a copy of the original element)
Best,
John
|
|
|
|
|
Something like this maybe?
public static SocketStructure[] GetHostAddressesList() {
lock(m_locker)
{
List<SocketStructure>tmp = new List<SocketStructure>tmp();
for(int i = 0; i<m_HostAddress.Count(); i++)
{
SocketStructure s = new SocketStructure();
s.IP = m_HostAddress[i].IP;
s.port = m_HostAddress[i].port;
tmp.Add(s);
}
return tmp.ToList();
}
}
|
|
|
|
|
Almost Note that string is immutable, so you're fine there.
public static List<SocketStructure> GetHostAddressesList() {
lock(m_locker)
{
List<SocketStructure>tmp = new List<SocketStructure>();
for(int i = 0; i<m_HostAddress.Count(); i++)
{
SocketStructure s = new SocketStructure();
s.IP = m_HostAddress[i].IP;
s.port = m_HostAddress[i].port;
tmp.Add(s);
}
return tmp;
}
}
At the last line, you don't need to use .ToList(), since you've already created a new list.
Best,
John
|
|
|
|
|
Ok, so you recommend I use this one? Btw tmp isn't array type above
|
|
|
|
|
Yes, I do. About array - yes you're right. Just modified it now.
|
|
|
|
|
So when I am returning value types - it is fine? (from the point of view of multithreading). And assuming ALL my getters (both for value and reference types) have locks.
But if I return reference type I better make a deep copy, that is better practice you say right?
|
|
|
|
|
You nailed it
Best,
John
|
|
|
|
|
Thanks - I hope we understood each other
PS. (Same applies for strings I guess as it does with ints, since string are immutable)
PPS Just started thinking indeed returning reference to object even if you have locks inside getters is rather tricky - because some other thread might change contents of the reference type, and if you are left with the reference which you retrieved above you will see different results depending how other thread modifies it
|
|
|
|
|
Member 12061600 wrote: PS. (Same applies for strings I guess as it does with ints, since string are immutable)
Yes
Member 12061600 wrote: PPS Just started thinking indeed returning reference to object even if you have locks inside getters is rather tricky - because some other thread might change contents of the reference type, and if you are left with the reference which you retrieved above you will see different results depending how other thread modifies it
Yes, it's very tricky indeed. So I would suggest you go with the easy solution for now (immutable objects or structs)
Best,
John
-- LogWizard - Meet the Log Viewer that makes monitoring log files a joy!
|
|
|
|
|
Member 12061600 wrote: So when I am returning value types - it is fine?
Yes
Member 12061600 wrote: But if I return reference type I better make a deep copy, that is better practice you say right?
It all depends on what you're trying to achieve, but if you're not sure about it, it's safe to assume "yes". But as said in the other answer, I suggest you go for immutable objects or structs.
Best,
John
-- LogWizard - Meet the Log Viewer that makes monitoring log files a joy!
|
|
|
|
|
I already went with deep copy approach I showed you above (the code). So I will leave it like that?
Because I have classes instead of structs and they aren't immutable.
|
|
|
|
|
It's up to you. You can also create a struct used just for you clients, something like:
struct socket {
string ip;
int port;
}
and return a list of that.
Best,
John
-- LogWizard - Meet the Log Viewer that makes monitoring log fies a joy!
|
|
|
|
|
Thanks but if there is no difference, I will leave it with deep copy, since I already implemented that.
|
|
|
|
|
1. Sounds good
2. It's pretty much another way to deep copy. But also, if you create a new struct, you can decide what your client has access to - at this point, it has access to all members of SocketStructure. With a struct, you would decide exactly what your client sees.
It the case of a simple structure (such as your SocketStructure), having a struct would probably be overkill, but for more complex classes, it might be a solution.
Best,
John
|
|
|
|
|
2. It's pretty much another way to deep copy. But also, if you create a new struct, you can decide what your client has access to - at this point, it has access to all members of SocketStructure. With a struct, you would decide exactly what your client sees.
Not sure I understood that - maybe because I am not very experienced with .NET yet. But if from thread safety point of view there is no difference whether I do a deep copy like I showed in my code, or return copied structs, I will stick to my approach then ...
It is ok if client has access to all members ...
|
|
|
|
|
Member 12061600 wrote: I will stick to my approach then ...
Sounds good
Best,
John
|
|
|
|
|
|
You're welcome!
Best,
John
-- LogWizard Meet the Log Viewer that makes monitoring log files a joy!
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
