CODEWITHSUNDEEP
c#.netstringmultithreading
Royi Namir
Royi Namir

Reputation: 148694

String.Concat and thread safety?

After asking this question , I still have a question.

I got both excellent answers but I'm still having a trouble finding how could this actually be occur: ( how could a fault situation can occur) :

I will start with an example : 

public void Do(string [] g)
{
   g=null;     //<========
}

void Main()
{
    var t=new string[3];
    t[0]="1";   t[1]="1";   t[2]="1";
    Do( t);
    Console.WriteLine ( t.Length);
}

the notated line can be execute in a different thread but each thread has its own g variable !
(and please remember that I can't add items to an arrray. cause array length is created at creation)

No matter what will I do with the function Do - (no matter in what thread) , the Console.Writeline result will always be 3 ( unless using ref).

so lets see the real code : 

    public static string Concat(params string[] values)
  #1  {
  #2      if (values == null)
  #3       {
  #4            throw new ArgumentNullException("values");
  #5       }
  #6        int totalLength = 0;
  #7        string[] strArray = new string[values.Length];
  #8        for (int i = 0; i < values.Length; i++)
  #9          {
  #10            string str = values[i];
  #11            strArray[i] = (str == null) ? Empty : str;
  #12            totalLength += strArray[i].Length;
  #13            if (totalLength < 0)
  #14            {
  #15                throw new OutOfMemoryException();
  #16            }
  #17         }
  #18        return ConcatArray(strArray, totalLength);
  #19   }

My saying is : once I'm on #1 , in thread X , this thread will forever will have array with length 3.

If another thread wants to destroy/change the array length ( which I cant understand how , cause the array has fixed length , all he can do is making it null) - it will have a different copy of pointer address.

I must be missing something here.

Upvotes: 0

Views: 1655

Answers (5)

Douglas
Douglas

Reputation: 37771

Array references are passed into the method, so each thread (in this case) would have its own reference g. Each thread can change what its own reference points to (eg, g = null) however when you access items in g, eg g[0], both threads will access the same item.

So, the problem is g[0] = null, not g = null.

Upvotes: 2

Henk Holterman
Henk Holterman

Reputation: 273591

what is the code which other thread could execute which will cause an error ?

string[] data = { "1", "2", "3" };
ThreadPool.QueueUserWorkItem( () => { data[0] = "one"; } );
string total = String.Concat(data);

Without the intermediate array there would be a race condition, the Thread could change data[0] after its length was added to totalLength but before the (presumably unsafe) call to ConcatArray(). The low-level method would then copy 5 chars to a buffer of size 3.

Upvotes: 4

Alois Kraus
Alois Kraus

Reputation: 13545

What John Skeet and Eric Lippert meant was that the calcuation of the total string length can lead to different results if the array contents can change after the length has been calculated. Because the concat operation is done in unmanaged code buffer overuns can occur if the string length afterwards did change.

To illustrate the issue here is a repro of the issue they were talking about:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

namespace ConsoleApplication1
{
    class Program
    {
        static string[] arr = new string[] { "1", "2", "3" };
        static string[] arr2 = new string[] { "1111", "2222222", "2222222222222222222222" };

        [MTAThread]
        public static void Main()
        {
            ThreadStart[] funcs = Enumerable.Range(0, Environment.ProcessorCount * 4).Select(i => new ThreadStart(MutateArray)).ToArray();
            foreach (var func in funcs)
                new Thread(func).Start();
            Console.ReadLine();
        }

        static void MutateArray()
        {
            Random rand = new Random();
            while (true)
            {
                int i = rand.Next(arr.Length);
                // swap array contents with contents from another array so arr will always
                // contain a mixture of arr and arr2 without knowing at which point of time which contents
                // are inside it.
                lock (arr)
                {
                    string tmp = arr[i];
                    arr[i] = arr2[i];
                    arr2[i] = tmp;
                }

                Do(arr); 

            }
        }

        static void Do(string[] g)
        {
            AllocateBufferWithRightLength(g, StrLen(g));
        }

        static void AllocateBufferWithRightLength(string[] g, int strLen)
        {
            int newLen = StrLen(g);
            if (strLen != newLen)
            {
                throw new Exception("Invariant broken");
            }
        }

        static int StrLen(string[] g)
        {
            int strLen = 0;
            foreach (var str in g)
            {
                if (str != null)
                    strLen += str.Length;
            }
            Thread.Sleep(1);
            return strLen;
        }



    }
}

Upvotes: 1

Avner Shahar-Kashtan
Avner Shahar-Kashtan

Reputation: 14700

I went over Jon Skeet and Eric Lippert's answers in the original thread, and I believe you're misinterpreting their answers. They're worried that if you don't copy the strings to a new array, someone will come along and change {"1","2","3"} to {"a very very", "long string", "(but much longer, yes?"} between the time ConcatString first evaluates the input and performs the concatenation.

Note that line 12 doesn't deal with the length of the input array, but with the length of the final output string. All these checks deal with the results of concetenation, not the input array.

Upvotes: 6

Tilak
Tilak

Reputation: 30728

The following lines ensures data consistency in case the array content is modified in other threads

  #11            strArray[i] = (str == null) ? Empty : str;
  #12            totalLength += strArray[i].Length;
  #18        return ConcatArray(strArray, totalLength);

Race is still there but before line no. 11, But if original array is used (in case of data corruption) wrong information may be passed to ConcatArray function.

Upvotes: 1

Related Questions