Why does this method spit out the same results to different users

So I have method that generates a barcode and label with that barcode. We have noticed that when more than one user generate a barcode at the same time they will both get the same file. I am using ASP.NET and I am hosting the application and the file on the a in house server.

public void trickylabel(string fnsku, string title)
{
    Random random = new Random();
    int randomNumber = random.Next(0, 100000);

    //Set barcode properties...

    code.parse(fnsku); // Text

    BCGDrawing drawing = new BCGDrawing(this.Server.MapPath("~") + "image"+ randomNumber.ToString() +".png", color_white);
    drawing.setBarcode(code);
    drawing.draw();

    // Draw (or save) the image into PNG format.
    Response.ContentType = "image/png";
    drawing.finish(ImageFormat.Png);

    Document doc = new Document(new iTextSharp.text.Rectangle(200f, 75f), 20F, 10F, 10F, 1F);
    PdfWriter writer = PdfWriter.GetInstance(doc, new FileStream(Request.PhysicalApplicationPath +
        "\\"+randomNumber.ToString()+".pdf", FileMode.Create));

    doc.Open();

    iTextSharp.text.Image png = iTextSharp.text.Image.GetInstance(this.Server.MapPath("~") + "开发者_运维问答image" + randomNumber.ToString() +".png");

    doc.Add(png);

    //Sets pdf properties...

    doc.Add(new Paragraph(title, times));
    PdfAction action = new PdfAction(PdfAction.PRINTDIALOG);
    writer.SetOpenAction(action);
    doc.Close();

    Response.ContentType = "application/pdf";
    Response.AppendHeader("Content-Disposition", "attachment; filename=labels.pdf");
    Response.TransmitFile(Server.MapPath("~/"+randomNumber.ToString()+".pdf"));

}

The problem is here:

Random random = new Random();
int randomNumber = random.Next(0, 100000);

The Random class generates pseudo-random sequence, that is based on some starting value, called Seed. If two Random instances are initialized with equal seeds they will produce equal number sequences.

When Random is created with parameterless constructor the Seed is created from current system time. If you ran a loop that creates instances of Random class you will notice that random.Next() value changes only several times per second (once per 16ms).

To avoid this you should reuse initialized Random instance (however, since you are using ASP.Net you will need to ensure that the calls of random.Next() are executed in a thread-safe way). Then your visitors will get different values each new random.Next(...) call.

Alternatively Random has a constructor that allows to set the initial Seed value manually. So, you can create your own algorithm of creating unique seeds for your users.

UPDATE Thread-safe Randomizer implementation. Gets initialized on assembly load.

Usage: just replace random.Next(0, 100000); to Randomizer.Next(0, 100000);

public static class Randomizer
{
    private static Random rnd;
    static Randomizer()
    {
        rnd = new Random();
        rndlock = new object();
    }

    private static object rndlock;
    public static int Next(int minValue, int maxValue)
    {
        lock(rndlock)
        {
            return rnd.Next(minValue, maxValue);
        }
    }
}

UPDATE On thread safety and unpredictable results

As stated above, Random generates pseudo-random sequence. This means that each int number in this sequence has known previous and next numbers. This means that after a certain number of calls these numbers will start to repeat. The Random.Next() algorithm is designed to maximize number of unique items in this sequence.

So, what "unpredictable" means in this case? When several threads use the same variables simultaneously the logic of the algorithm becomes corrupted. In best scenario this makes sequences of same numbers occur more often. In worst scenario Random.Next() starts to produce zero values on each call without any chance to recover.

More useful information about System.Random on MSDN.

Or just use NewGuid Method

If the requests are starting at the same time, then the Random numbers that are produced may well be the same as each other. Without providing a seed value, the Random constructor takes its seed based on the system clock, so it is possible for two instances to produce the same set of random values (which aren't random, they're pseudo-random).

Documentation here - http://msdn.microsoft.com/en-us/library/system.random.aspx

Try making your Random instance a static member of the class and having your method just call .Next on that member.

Do you need to save the pdf to disk?

I'd suggest saving the png image and the pdf document to a MemoryStream. This might also lead to better performance, because there is less disk access.

public void trickylabel(string fnsku, string title)
{
    //Set barcode properties...

    code.parse(fnsku); // Text

    BCGDrawing drawing = new BCGDrawing(color_white);
    drawing.setBarcode(code);
    drawing.draw();

    // Draw (or save) the image into PNG format.

    using (var imageStream = new MemoryStream())
    {
        drawing.finish(ImageFormat.Png, imageStream);

        using (var result = new MemoryStream())
        {
            Document doc = new Document(new iTextSharp.text.Rectangle(200f, 75f), 20F, 10F, 10F, 1F);
            PdfWriter writer = PdfWriter.GetInstance(doc, result);

            doc.Open();

            var png = iTextSharp.text.Image.GetInstance(System.Drawing.Image.FromStream(imageStream), ImageFormat.Png);
            doc.Add(png);

            //Sets pdf properties...

            doc.Add(new Paragraph(title, times));
            PdfAction action = new PdfAction(PdfAction.PRINTDIALOG);
            writer.SetOpenAction(action);
            doc.Close();

            Response.ContentType = "application/pdf";
            Response.AppendHeader("Content-Disposition", "attachment; filename=labels.pdf");
            Response.BinaryWrite(result.ToArray());
        }
    }
}