File not closing properly in C#

khyer123
khyer123 used Ask the Experts™
on
I am working on a C# application, that involves creating a text file (commands.txt), filling it with some text, closing the file, and then have a different program read the file. This happens after a user fills out some information, and then clicks a button. It works great the first time. However, if the user clicks the button a second time, the program it executes (in this case, plink, a command line version of putty) says that it can't open my 'commands.txt' file. I have to exit out of the program, and re-open it in order to get it to work again.

I closed the writer, and the stream as you can see in the code below.

Any ideas?

//create command file
string command_path = txt_plink7.Text.Replace("plink.exe", "commands.txt");
FileStream cmd_stream = new FileStream(command_path, FileMode.Create);
StreamWriter cmd_writer = new StreamWriter(cmd_stream);
cmd_writer.WriteLine("sh zoneset active | inc \"pwwn 20:|pwwn 21:\"");
Application.DoEvents();
cmd_writer.Close();
cmd_stream.Close();
Application.DoEvents();

Open in new window

Comment
Watch Question

Do more with

Expert Office
EXPERT OFFICE® is a registered trademark of EXPERTS EXCHANGE®
Todd GerbertIT Consultant
Top Expert 2010

Commented:
Is it possible plink is keeping a lock on the commands.txt file?

Author

Commented:
I don't think so - I ran a test, and C# even has the ability of deleting commands.txt, and then recreating it, and I still have this problem. Also, the batch file has plink run several times - once for each hostname in the list
I do not know what the rest of your application looks like, but I suspect that this is running in a very tight loop (like a for-next). There is a chance that the Framework does not get a chance to garbage-collect (and hence release) all resources allocated to the FileStream.

Try adding cmd_stream.Dispose( true ); after line #9.
Introduction to R

R is considered the predominant language for data scientist and statisticians. Learn how to use R for your own data science projects.

Author

Commented:
NotLogical, you are probably right - when I tried adding cmd_stream.Dispose(true), and I get a compiler error: System.IO.Stream.Dispose(bool) is inaccessible due to its protection level(CS0122)
Hi,

Sorrry... Looked at wrong code-completion object...

Try:

   cmd_stream.Dispose();

If you want to limit the lifetime (or scope) of an object, you can use the "using" statement as below. Once the code exits the "using" block, the object is disposed.

            //create command file
            string command_path = txt_plink7.Text.Replace("plink.exe", "commands.txt");
            using (FileStream cmd_stream = new FileStream(command_path, FileMode.Create))
            {
                StreamWriter cmd_writer = new StreamWriter(cmd_stream);
                cmd_writer.WriteLine("sh zoneset active | inc \"pwwn 20:|pwwn 21:\"");
                Application.DoEvents();
                cmd_writer.Close();
                cmd_stream.Close();
                Application.DoEvents();
            }

Open in new window

Author

Commented:
It compiled, but I am still having the same problem
Could you outline what happens just outside of this code block?

Author

Commented:

			//open the output file
			FileStream output_stream;
			StreamWriter output_writer;
			open_output:
			try
			{
				output_stream = new FileStream(txt_output7.Text, FileMode.Create);
				output_writer = new StreamWriter(output_stream);
			}
			catch
			{
				DialogResult result = MessageBox.Show("There was an error opening the hostnames file. Check to make sure that the file is not open, and that it exists.", "Error", MessageBoxButtons.RetryCancel, MessageBoxIcon.Error);
				if(result == DialogResult.Retry)
				{
					goto open_output;
				}
				else
				{
					return;
				}
			}
			
			
			//open the batch file
			string batch_path = txt_plink7.Text.Replace("plink.exe", "batch.bat");
			FileStream batch_stream = batch_stream = new FileStream(batch_path, FileMode.Create);
			StreamWriter batch_writer =  new StreamWriter(batch_stream);
					
			
			//create command file
			/*
			FileStream cmd_stream = new FileStream(command_path, FileMode.Create);
			StreamWriter cmd_writer = new StreamWriter(cmd_stream);
			cmd_writer.WriteLine("sh zoneset active | inc \"pwwn 20:|pwwn 21:\"");
			Application.DoEvents();
			cmd_writer.Close();
			cmd_stream.Close();
			Application.DoEvents();
			cmd_stream.Dispose( );
			*/
			
			string command_path = txt_plink7.Text.Replace("plink.exe", "commands.txt");
 
			using(FileStream cmd_stream = new FileStream(command_path, FileMode.Create))
			{
			StreamWriter cmd_writer = new StreamWriter(cmd_stream);
			cmd_writer.WriteLine("sh zoneset active | inc \"pwwn 20:|pwwn 21:\"");
			Application.DoEvents();
			cmd_writer.Close();
			cmd_stream.Close();
			Application.DoEvents();
			}
			
			//get the contents of the Onaro CSV File
			ArrayList onaro = new ArrayList();
			input_reader.ReadLine(); //skip the first line
			while(!input_reader.EndOfStream)
			{
				string line = input_reader.ReadLine();
				string[] line_split = line.Split(',');
				if(line_split.Length > 1)
				{
					string wwn = line_split[1];
					if(wwn.Length>1) //sometimes the pwwn is blank in the onaro export file
					{
						string name = line_split[3];
						if(name.Length<2)
						{
							name = "unknown";
						}
						string[] wwn_split = wwn.Split(' '); //sometimes there are multiple PWWN's listed for a WWN
						{
							int wwn_length = wwn_split.Length;
							for(int i = 0; i < wwn_length; i++)
							{
								pwwn x = new pwwn();
								x.wwn = wwn_split[i];
								x.name = name;
								onaro.Add(x);
							}
						}
						
					}
				}
				
			}

Open in new window

Mike TomlinsonHigh School Computer Science, Computer Applications, Digital Design, and Mathematics Teacher
Top Expert 2009

Commented:
Does it make any difference this way?

            string command_path = txt_plink7.Text.Replace("plink.exe", "commands.txt");
            System.IO.File.WriteAllText(command_path, "sh zoneset active | inc \"pwwn 20:|pwwn 21:\"");
            Application.DoEvents();

Author

Commented:
No, it still didn't work
Hi,

Here is something very interesting. I tried duplicating your environment, and using plink.exe to read that command file and send the commands to a dummy server.

In order to try and make the code fail, I added a "while( true );" at different points - effectively causing the code to deadlock - and I could read the contents of the file with plink at any time! Even with the file still open from the C# code.

In fact, one way to test the code to see whether the file is actually properly flushed and closed, is to delete it. When I insert the deadlock after the end of the "using" block, I can delete the file without problems. This means that the file is indeed flushed and closed. However, if I attempt to do so earlier, I get a "The process cannot access the file because it is being used by another process." which is expected.

Here is a question: at which point in time do you execute plink? Could there be a condition where plink is executing before the contents of "commands.txt" is actually created (or is being updated)? Can you add a small delay before launch to see if this could be the case?

Thanks...

Author

Commented:
Looking at the second example of code I sent, the batch file that is created comes right after that, and then the process is launched. One of the troubleshooting steps I took was to to Thread.Sleep(5000) right after the commands.txt file was created, I thought there was a delay in the write as well.

Also, it's currently set to remove the commands.txt file after the batch file is finished, so every time the run button is clicked, it creates a new file with that same information.

Is Thread.Sleep sufficient for a delay? It freezes the entire program - is there a command that lets the program pause, but not completely freeze up?
Thread.Sleep causes the currently executing thread to pause. In the case of a Windows Form, there is only a single thread of execution. This thread updates the UI and handles all user interaction. Thus, when you Sleep() this thread, it will cause the UI to become unresponsive.

You could put your processing onto a background thread without too much difficulty, and I can show you how. Are you sure you want to get away from the original problem, though? :^)

I was thinking of a similar scenario: could the time that it takes the batch file to start up and plink to read the commands.txt file, the file was getting removed?

Could you add a little troubleshooting to your batch file and see if a directory listing will show the file as being there? Can you "type commands.txt" in your batch file and see that the commands are present?

Commented:
if you are generate same file name each tiem then also a problem. b'coz file  already exists.


            FileStream cmd_stream = new FileStream(command_path, FileMode.CreateOrOpen)

Open in new window

Author

Commented:
NotLogical - I will give your suggestion a shot on Monday to see if the file is actually there when plink is getting called. One other thing to note that I found interesting, is that once it starts saying that plink can't find the commands file, no matter now many times I try after that, and no matter how long I wait to run the function again, it won't work until I restart the program. Thank you for all of your help on this by the way :)

jinal - the file gets deleted after the function is run, so when it's run again, it no longer exists
Todd GerbertIT Consultant
Top Expert 2010
Commented:
Are you sure your button click event handler is ever finishing?  If it's not, e.g. if the plink.exe process is holding it up somehow, a second click of the button would cause two routines to both be attempting to read/write the same commands and batch files.

Is it acceptable to you to use temporary file names for the commands and batch files?  This way you'll definitely avoid file access collisions - just delete both files at the end of the event handler.


// string batch_path = txt_plink7.Text.Replace("plink.exe", "batch.bat");
string batch_path = Path.GetTempPath() + Path.GetRandomFileName() + ".bat";
 
// string command_path = txt_plink7.Text.Replace("plink.exe", "commands.txt");
string command_path = Path.GetTempPath() + Path.GetRandomFileName() + ".txt";

Open in new window

Commented:
But it is better to check file before creation.

Like

If (file.Exists())
{
     file.Delete();
}

Sometime if you debug and process stop in between after creation of file so file can not deleted. So always better to check first then create new one after delete.
Todd GerbertIT Consultant
Top Expert 2010

Commented:
I disagree.  It's temporary information, it should go in a temporary file.

What happens if the application hangs while it has a lock on the file, and the user launches a second instance?

If "new Stream(path, FileMode.Create)" is unable to overwrite the file why would you expect File.Delete() to succeed?
Todd GerbertIT Consultant
Top Expert 2010

Commented:
khyer123:

How are you launching your batch file?  Does your button's click event handler wait for the batch file to complete before returning?

Author

Commented:
tgerbert: The batch file is launched by creating a process, launching the process (the batch file), and waiting until it's finished before returning (it processes some data that the batch file pulls into a temporary text file before returning).
@khyer123:

Did you have any luck with adding the extra troubleshooting bits to your batch file?

Also, could you check something for me, using Task Manager: launch your application, click the button, and observe the processes getting created. When plink finishes, ensure that all of the processes which were created, actually disappear. Is an instance of plink still hanging around, perhaps?

In all of the troubleshooting so far, I do not recall seeing the code which launches your external process... Did I just overlook that somewhere? :^)

Cheers,

NotLogical

Author

Commented:
@NotLogical

So I put "type commands.txt" in the batch file, and sure enough, when putty fails, it's because the system can't read the file: "Tye system cannot find the file specified".

I'm going to give the temporary file method a shot.

Author

Commented:
Oh, and here is the code including the process call
			//open the batch file
			string batch_path = txt_plink7.Text.Replace("plink.exe", "batch.bat");
			FileStream batch_stream = batch_stream = new FileStream(batch_path, FileMode.Create);
			StreamWriter batch_writer =  new StreamWriter(batch_stream);
					
			
			//create command file
			/*
			FileStream cmd_stream = new FileStream(command_path, FileMode.Create);
			StreamWriter cmd_writer = new StreamWriter(cmd_stream);
			cmd_writer.WriteLine("sh zoneset active | inc \"pwwn 20:|pwwn 21:\"");
			Application.DoEvents();
			cmd_writer.Close();
			cmd_stream.Close();
			Application.DoEvents();
			cmd_stream.Dispose( );
			*/
			
			string command_path = txt_plink7.Text.Replace("plink.exe", "commands.txt");
			System.IO.File.WriteAllText(command_path, "sh zoneset active | inc \"pwwn 20:|pwwn 21:\"");
			Application.DoEvents();
			
			/*
			using(FileStream cmd_stream = new FileStream(command_path, FileMode.Create))
			{
			StreamWriter cmd_writer = new StreamWriter(cmd_stream);
			cmd_writer.WriteLine("sh zoneset active | inc \"pwwn 20:|pwwn 21:\"");
			Application.DoEvents();
			cmd_writer.Close();
			cmd_stream.Close();
			Application.DoEvents();
			}
		*/	
			//get the contents of the Onaro CSV File
			ArrayList onaro = new ArrayList();
			input_reader.ReadLine(); //skip the first line
			while(!input_reader.EndOfStream)
			{
				string line = input_reader.ReadLine();
				string[] line_split = line.Split(',');
				if(line_split.Length > 1)
				{
					string wwn = line_split[1];
					if(wwn.Length>1) //sometimes the pwwn is blank in the onaro export file
					{
						string name = line_split[3];
						if(name.Length<2)
						{
							name = "unknown";
						}
						string[] wwn_split = wwn.Split(' '); //sometimes there are multiple PWWN's listed for a WWN
						{
							int wwn_length = wwn_split.Length;
							for(int i = 0; i < wwn_length; i++)
							{
								pwwn x = new pwwn();
								x.wwn = wwn_split[i];
								x.name = name;
								onaro.Add(x);
							}
						}
						
					}
				}
				
			}
			
			input_reader.Close();
			input_stream.Close();
			
			//get the information from the switches
			string tempoutput_path = txt_plink7.Text.Replace("plink.exe", "tempoutput.txt");
			while(!host_reader.EndOfStream)
			{
				string line = host_reader.ReadLine();
				if(line.Length>0)
				{
					batch_writer.WriteLine("echo HOST "+line+">>\""+tempoutput_path+"\"");
					batch_writer.WriteLine("type commands.txt");
					string cmd = "\"" + txt_plink.Text+ "\" -ssh -pw " + txt_pass7.Text + " -noagent -m commands.txt " + txt_user7.Text + "@"+line+">>\""+tempoutput_path+"\"";
					batch_writer.WriteLine(cmd);					
				}
			}
			host_reader.Close();
			host_stream.Close();
			
			batch_writer.WriteLine("pause");
			batch_writer.Close();
			batch_stream.Close();
		    Application.DoEvents();
			Process p = new Process();
			p.StartInfo.FileName = batch_path;
			p.Start();
			p.WaitForExit();
			
			File.Delete(batch_path);
			File.Delete(command_path);
			

Open in new window

Todd GerbertIT Consultant
Top Expert 2010

Commented:
It looks like your commands.txt only has one line it, and that never changes.  If that's the case, you can skip that file completely and put the commands on the command line to plink.

Change line 80, from string cmd = "\"" + txt_plink.Text+ "\" -ssh -pw " + txt_pass7.Text + " -noagent -m commands.txt " + txt_user7.Text + "@"+line+">>\""+tempoutput_path+"\"";

to

cmd = String.Format("\"{0}\" -ssh -pw {1} -noagent {2}@{3} \"{4}\" >> \"{5}\"",
                                 txt_plink.Text, txt_pass7.Text, txt_user7.Text, line, tempoutput_path);

Author

Commented:
@tgerbert: I tried that earlier, but plink didn't like it because there are quotes in the command.

Author

Commented:

Alright, it looks like the temporary file solution finally worked! Thank you, both tgerbert and NotLogical for your assistance. I'm going to split the points between you guys: tgerbert for providing the solution, and NotLogical for the other suggestions that ended up cleaning up some of my code, and for the type command to show that it wasn't necessarily plink's fault :)

Thanks again,
Ken

Author

Commented:
Thanks, guys!

Author

Commented:
Also, I'd like to clarify for anyone else who might run into this problem:

string batch_path = Path.GetTempPath() + Path.GetRandomFileName() + ".bat";
should just be
string batch_path = Path.GetRandomFileName() + ".bat";

GetRandomFileName() also got the path for me.
Todd GerbertIT Consultant
Top Expert 2010

Commented:
You need to double the backslashes before the quotes.

The code below would result in:

c:\program files\putty\plink.exe -ssh -pw thePassword -noagent user@host "sh zoneset active | inc \\"pwwn 20:|pwwn 21:\\""
cmd = String.Format(@"""{0}"" -ssh -pw {1} -noagent {2}@{3} ""sh zoneset active | inc \\""pwwn 20:|pwwn 21:\\"""" >> ""{4}""",
                    txt_plink.Text, txt_pass7.Text, txt_user7.Text, line, tempoutput_path);

Open in new window

Todd GerbertIT Consultant
Top Expert 2010

Commented:
GetTempFileName() will return a complete path, GetRandomFileName() does not (at least not on my system).  The reason I used GetRandomFileName() is because it only returns a file name; GetTempFileName() not only returns the path to the file, but also creates a zero-byte file on disk.  Thus, this code:

string tempFile = Path.GetTempFileName() + ".bat";
File.WriteAllText(tempFile, "Hello World");
File.Delete(tempFile);

Would create two files: abcd123.tmp and abcd123.tmp.bat, and then only delete the .bat one leaving an empty file in the temp folder (which might eventually add up over time).  You would need to rename the temp file to include the .bat extension, or use GetRandomFileName() (which does not create a file).

string tempFile = Path.GetTempFileName();
File.Move(tempFile, tempFile + ".bat");
....etc

Author

Commented:
Interesting, when I did it on my machine, GetTempFileName() returned the entire path, including the file name. I got an error when I tried it the other way because it was doing C:\documents and settings\user\temp\C:\documents and settings\user\temp\<filename>.tmp

I do have it delete both files when the function is done with them.

In any case, I will also keep the one-line command plink syntax in mind as well. My next step is to try and figure this out using some sort of SSH library instead of using Plink, but I needed to get this out quickly.

Thanks again for all of your help
Todd GerbertIT Consultant
Top Expert 2010

Commented:
Correct - GetTempFileName() does indeed return the entire path, GetRandomFileName() does not (they're two different methods).

Author

Commented:
Ohhhhh I see what I did - alright that makes sense then.

Do more with

Expert Office
Submit tech questions to Ask the Experts™ at any time to receive solutions, advice, and new ideas from leading industry professionals.

Start 7-Day Free Trial