none
Stream.CopyTo() - Network stream to file stream hang

    Question

  • I have a issue with my TCP-over-network file transfer mechanics. Basically what's happening is, theClient (File sender) and the Server (File receiver) communicate via a simple message system.

    The Client initiates the transfer by sending a message that contains a Send command, followed by the length of the Filename, followed by the actual Filename. The Server parses the message and lets the user decide whether he/she wants to Accept or Reject the file. The appropriate message is then sent back to the Client. If the Accept command was read by the Client, the file transfer is initialized. This part completes successfully using either the Stream.CopyTo() method or via my custom solution.

    This is also where the problem occurs. The Server doesn't move past that line of code (code shown below), the CopyTo(), just sits there indefinitely but when I close the application, the file gets transfered succesfully. Could be some threading issues or so, I'm not sure. 

    Regarding threading

    Both of the methods are started in their own, separate threads, as shown below.

    Thread t = new Thread(StartListening);
    t.IsBackground = true;
    t.Start();
    
    if (!String.IsNullOrEmpty(_path))
    {
        var t = new Thread(SendFile);
        t.IsBackground = true;
        t.Start();
    }
    else
    {
        MessageBox.Show("You have to choose a file!", "File error!", MessageBoxButtons.OK, MessageBoxIcon.Error);
    }

    StartListening 

    private void StartListening()
        {
            _listener = new TcpListener(_localEndPoint);
            _listener.Start();
    
            try
            {
                while (!done)
                {
                    // Buffer for reading.
                    byte[] buffer = new byte[4096];
                    int bytesRead;
    
                    SetText("SERVER : Listening for connections...\r\n");
                    using (TcpClient client = _listener.AcceptTcpClient())
                    {
                        SetText("SERVER : A client connected!\r\n");
                        using (NetworkStream netStream = client.GetStream())
                        {
                            SetText("SERVER : Waiting for the initial message...\r\n");
                            bytesRead = netStream.Read(buffer, 0, buffer.Length);
    
                            // Create a new Message based on the data read.
                            var message = new Message(buffer);
    
                            // Ask the user whether he/she wants to accept the file.
                            DialogResult dr = MessageBox.Show("Do you want to accept this file : " + message.Filename, "Accept or reject?", MessageBoxButtons.OKCancel);
    
                            // If the user says yes, send the accept response and start accepting the file data.
                            if (dr == DialogResult.OK)
                            {
                                SetText("SERVER : The user accepted the file! Sending the accept response and ready for transfer...\r\n");
    
                                // The Message class static methods for transforming commands into byte arrays.
                                byte[] responseBytes = Message.ConvertCommandToBytes(Commands.Accept);
    
                                // Send the accept response.
                                netStream.Write(responseBytes, 0, responseBytes.Length);
    
                                // Open or create the file for saving.
                                using (FileStream fileStream = new FileStream((@"E:\" + message.Filename), FileMode.Create))
                                {
                                    SetText("Before CopyTo()\r\n");
    
                                    // Copy the network stream to the open filestream. "DefaultBufferSize" is set to the "short.MaxValue"
                                    // !!!!!!!!!!!!!!!!!
                                    // This line never ends, it gets stuck on this line.
                                    // !!!!!!!!!!!!!!!!!
                                    netStream.CopyTo(fileStream, DefaultBufferSize);
    
                                    SetText("After CopyTo()\r\n");
    
                                    // Check whether the file was transfered (will add more precise checks).
                                    if (File.Exists(@"E:\" + message.Filename))
                                        _fileCopied = true;
                                }
                            }
                            // If the user rejected the transfer, send the Reject response.
                            else
                            {
                                SetText("SERVER : The user rejected the file! Sending reject response...\r\n");
                                byte[] responseBytes = Message.ConvertCommandToBytes(Commands.Reject);
                                netStream.Write(responseBytes, 0, responseBytes.Length);
                            }
                        }
                    }
    
                    // If the file was successfully transfered, send the Success message notifying the client that
                    // the operation ended successfully.
                    if (_fileCopied)
                    {
                        DialogResult dr = MessageBox.Show("Do you want to open the directory where the file was saved?",
                            "Confirmation", MessageBoxButtons.OKCancel);
    
                        if (dr == DialogResult.OK)
                            Process.Start(@"E:\");
                    }
                }
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.ToString());
            }
        }

    Send File 

    try
            {
                using (TcpClient client = new TcpClient())
                {
                    SetText("CLIENT : Connecting to the host...\r\n");
    
                    // Attempt to connect to the Server
                    client.Connect(_remoteEndPoint);
    
                    SetText("CLIENT : Connected to the host!\r\n");
    
                    using (NetworkStream netStream = client.GetStream())
                    {
                        // The Message class has a constructor for the initial message. It just needs
                        // the Filename and it will construct the initial message that contains the
                        // [Send] command, file length and the actually filename.
                        Message message = new Message(_filename);
    
                        // Convert the message to a byte array.
                        byte[] messageBytes = message.ToBytes();
    
                        SetText("CLIENT : Sending the initial message!\r\n");
    
                        // Send the initial message to the server.
                        netStream.Write(messageBytes, 0, messageBytes.Length);
                        SetText("CLIENT : Initial message sent! \r\n");
                        SetText("CLIENT : Waiting for the response...\r\n");
    
                        // Wait for the response for the server. [Accept] or [Reject].
                        bytesRead = netStream.Read(buffer, 0, buffer.Length);
                        SetText(String.Format("CLIENT : Received the response - {0} bytes. Analyzing...\r\n", bytesRead));
    
                        // Try to convert the read bytes to a command.
                        Commands command = Message.ConvertBytesToCommand(buffer);
                        SetText("CLIENT : Received this response : " + command + "\r\n");
    
                        // Determine the appropriate action based on the command contents.
                        if (command == Commands.Accept)
                        {
                            SetText("CLIENT : The host accepted the request. Starting file transfer...\r\n");
    
                            // Open the chosen file for reading. "_path" holds the user specified path.
                            using (FileStream fileStream = new FileStream(_path, FileMode.Open))
                            {
                                // Initiate the file transfer.
                                fileStream.CopyTo(netStream, DefaultBufferSize);
                                SetText("CLIENT : Successfully sent the file to the host!\r\n");
                            }
    
                            // Wait for the [Success] or [Error] response.
                            netStream.Read(buffer, 0, bytesRead);
    
                            // Convert the bytes received to a command.
                            command = Message.ConvertBytesToCommand(buffer);
    
                            // Act appropriately.
                            if (command == Commands.Success)
                                MessageBox.Show("The host successfully received the file!");
                            else
                                MessageBox.Show("The transfer was unsuccessful!");
    
                        }
                        else if(command == Commands.Reject)
                        {
                            MessageBox.Show("The host rejected the transfer!");
                        }
                    }
                }
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.ToString());
            }
        }

    Set Text Callback

    public void SetText(string text)
        {
            if (statusBox.InvokeRequired)
            {
                SetTextCallback c = SetText;
                Invoke(c, new object[] {text});
            }
            else
            {
                statusBox.Text += text;
            }
        }
    
        private delegate void SetTextCallback(string text);

    In addition, since I'll have to make this fully async (I know a thread for a client connection's really bad practice), what would be the best way to accomplish this?

    I would greatly appreciate any assistance in figuring out the problem, buy ya a beer! :)

    Best regards,

    D6mi



    Monday, March 17, 2014 2:55 PM

Answers

  • "Secondly, the Client is the one that's sending the file and the Server is the file receiver, not sure if that's important to point out."

    Oops, I didn't notice that. Let's say sender and receiver for better clarity.

    The sender may use Stream.CopyTo. The receiver cannot use Stream.CopyTo unless the sender closes the socket after sending the file, your sender doesn't do that, it waits for a confirmation from the receiver that the file was received. The receiver never sends that confirmation because it is stuck in CopyTo waiting for more file data to arrive.

    "Allthough this works, I have a feeling this isn't a "proper" solution, like it's hacked together and won't withstand the test of time."

    Why would it be a hack? There are a few things that could be improved about this code (move the declaration of bytesRead inside the loop and use buffer.Length instead of DefaultBufferSize to avoid mistakes) but otherwise this is typical stream copying code.

    • Marked as answer by D6mi Tuesday, March 18, 2014 5:00 PM
    Tuesday, March 18, 2014 6:04 AM
    Moderator

All replies

  • Stream.CopyTo isn't suitable in this case. CopyTo tries to copy the entire stream, for such purposes a NetworkStream is an infinite stream so CopyTo will never get to its end. The only situation where this can work is when the server closes the connection after sending the file, then the client knows that the stream ended.

    In your case the server doesn't close the stream, it waits for a reply from the client but the client is still waiting for the file to be copied.

    Have the server send the file size before sending the actual file data so the client knows how much file data has to read. Of course, in this case you'll have to copy the stream manually, CopyTo can't be used on the client side.

    And beware with network reads and commands. You call Read and store its result in a variable but you never use that result. That result indicates how many bytes were actually read from the network and if you ignore it then you may end up using who knows what bytes happen to be in the buffer. Unless those commands are 1 byte in length there's a chance that Message.ConvertBytesToCommand will fail.

    "In addition, since I'll have to make this fully async (I know a thread for a client connection's really bad practice), what would be the best way to accomplish this?"

    If you don't have a high number of connections then using a thread per client is not so bad. In any case, it's much simpler than using async.

    Monday, March 17, 2014 3:23 PM
    Moderator
  • To start of, I'd like to thank you for your response :).

    Secondly, the Client is the one that's sending the file and the Server is the file receiver, not sure if that's important to point out. 

    Anyhow, let me see if I understand the problem : 

    The Client sends the file using CopyTo() and since "a network stream is an infinite stream so CopyTo will never get to its end" the Server never really gets past the CopyTo() line? 

    "The only situation where this can work is when the server closes the connection after sending the file, then the client knows that the stream ended?" - This would explain why the file gets transferred successfully when I close the application.

    In regards to your other points,

    • "And beware with network reads and commands. You call Read and store its result in a..."- I will most definitely make my application protocol more robust and bulletproof, just need to solve one issue at a time and this one really made me stuck. 
    • "If you don't have a high number of connections then using a thread per client is not so bad. In any case, it's much simpler than using async." - I just might do it on separate threads and try to go fully async if I find some time. 

    • Edited by D6mi Monday, March 17, 2014 5:06 PM Added text highlight for clarity and reading ease.
    Monday, March 17, 2014 4:13 PM
  • Got home and tried this on the receiving side, the sending side still uses CopyTo()

    using (FileStream fileStream = new FileStream((@"E:\" + message.Filename), FileMode.Create))
                                    {
                                        long remaining = message.FileLength;
                                        int bytesRead = 0;
                                        while (remaining > 0)
                                        {
                                            bytesRead = netStream.Read(buffer, 0, DefaultBufferSize);
                                            fileStream.Write(buffer, 0, bytesRead);
                                            remaining -= bytesRead;
                                        }        
                                    }
    Doesn't hang, successfully copies the file and receives the response from the server regarding the success of the operation. Allthough this works, I have a feeling this isn't a "proper" solution, like it's hacked together and won't withstand the test of time.


    Monday, March 17, 2014 10:40 PM
  • "Secondly, the Client is the one that's sending the file and the Server is the file receiver, not sure if that's important to point out."

    Oops, I didn't notice that. Let's say sender and receiver for better clarity.

    The sender may use Stream.CopyTo. The receiver cannot use Stream.CopyTo unless the sender closes the socket after sending the file, your sender doesn't do that, it waits for a confirmation from the receiver that the file was received. The receiver never sends that confirmation because it is stuck in CopyTo waiting for more file data to arrive.

    "Allthough this works, I have a feeling this isn't a "proper" solution, like it's hacked together and won't withstand the test of time."

    Why would it be a hack? There are a few things that could be improved about this code (move the declaration of bytesRead inside the loop and use buffer.Length instead of DefaultBufferSize to avoid mistakes) but otherwise this is typical stream copying code.

    • Marked as answer by D6mi Tuesday, March 18, 2014 5:00 PM
    Tuesday, March 18, 2014 6:04 AM
    Moderator
  • Everything understood loud and clear. I can't thank you enough :). Your second reply marked as the answer.

    Best regards,

    D6mi

    Tuesday, March 18, 2014 5:01 PM