|
I am kind of digging into this threading business and having some trouble. I created the threads (one for prefilling a bunch of information from a SQL database, and another one for inserting files into the database if the user chooses to do so). So the first on happens on load and the other one could happen at any time by either clicking the Attach button, or by dragging and dropping a file from the desktop to the listview.
Problem is when I spawn the thread and the user closes the form before the file is uploaded (the form is a child form and is not the parent main form), it gives an error about the control cannot be invoked when the handle is not created. OK, I understand that it has disposed of the forms controls and thats why it has done this... but how do I make sure these threads are stopped before disposing of all the components on the form?
Here is the method I used to pass the information about the file. So I call this method on the main thread and this methods sets the global variables (so the new thread can access them and have the information to insert into the database), show the progressbar, and then spawn the thread. As you can see the thread is not a declared globally so the dispose method cannot access it directly correct?
private void SaveFile(string fileName, string safeFileName, bool fileMR)
{
this.fileName = fileName;
this.safeFileName = safeFileName;
this.fileMR = fileMR;
if (fileMR)
picProgressFilesMR.Visible = true;
else
picProgressFiles.Visible = true;
Thread fileThread = new Thread(new ThreadStart(saveFiles));
fileThread.IsBackground = true;
fileThread.Start();
}
|
|
|
|
|
Hi,
there are different aspects involved:
1. FWIW: when a thread needs to update the GUI, you have to use Control.InvokeRequired/Control.Invoke [^]
2. A BackgroundWorker may or may not make life a little easier.
3. saving data to files probably should always run to completion, so I would not use a background thread here.
4. when a Form should not be closed yet because something it started (e.g. saving files) hasn't finished yet,
you could check for that in the FormClosing event, and inform/ask the user what to do, possibly setting Cancel true.
5. Aborting a thread is always a bad idea, you don't know in what state its objects are going to be left behind. A cooperative cancel is OK, however it isn't always feasible.
Jacob Dixon wrote: the thread is not a declared globally so the dispose method cannot access it directly correct?
wrong. An object is collectible as soon as it goes out of scope. A disposable object will have its finalizer (and hence Dispose) called by the GC when the GC determines it is collectible.
Thread objects are a bit special as MSDN says: It is not necessary to retain a reference to a Thread object once you have started the thread. The thread continues to execute until the thread procedure is complete.
Which I would rephrase as: a thread keeps itself alive and uncollectible until it reaches the end of its code.
|
|
|
|
|
Oh... In the method that I created the thread for it does use invoke.. Now while we are on that subject I've noticed there are multiple ways of invoking a control. The way I chose to do it required less lines of code in my opinion and without the need of created another method outside of this one:
Invoke((Action)(() => { labelFiles.Text = string.Format("Saving File... Size: {0}MB", Math.Round((fileSize / 1024) / 1024, 2)); }));
The way I have been doing it was like this:
private delegate void FilesLabelDelegate(string text);
private void FilesLabel(string text)
{
labelFiles.Tex = "blahblahblah";
}
Invoke(new FilesLabelDelegate(FilesLabel), new object[] { "blahblahblah" }));
I'm assuming that between the two isn't really a performance difference that would matter as much?
Anyways,
So before the thread starts I could set something saying that a file is currently being saved, and when the thread comes to an end I need to set that something back to false (like a variable). And on the FormClosing if that variable is true then display that we can't exit until the file is saved and call the e.Cancel = true;
That is a great idea thanks
|
|
|
|
|
1.
There are a couple of ways to correctly access the GUI from another thread; I'm strongly in favor of the one I call the "canonical form", as it works on all versions of C# and VB.NET
2.
yes your FormClosing needs to check your app's state and, when necessary, communicate with the user. How exactly is a design choice: "busy" and wait (  ); "busy" + cancel button; "busy" and ignore closing; simply ignore closing ( which makes it look like a bug).
my preference is to save files synchronously, without a thread; the one exception is when you can and often will start saving long before the user is likely to close the form. Anyway I as a user want to make sure things got saved before I drastically change the GUI state.
Rationale: saving what I entered/changed isn't taking as long as me entering/changing it, and I'm willing to spend that time to make sure and avoid the possibility I have to redo it.
|
|
|
|
|
Uhmm.. Ok I think I am going to go with a couple of you guys ideas put together. I think I might use a background worker that supports cancelling. If the bw is busy then I will ask the user if they want to continuing waiting until it is finished or stop the current process and close.
The reason I want to use it on a seperate thread is because it is a inventory screen. Often we must go in and enter updated information (which can be a lot) and save files. I don't really want saving a file to freeze the GUI when they could start saving the file and continue working on something else.
|
|
|
|
|
Things to keep in mind:
- cancelling a BGW (or any threaded action) requires cooperation; if there is only one big action (one SQL statement executing), then cancelling means waiting for completion.
- the "continue working on something else" may encounter a DB table that is being updated and possibly inconsistent.
Also I'm not convinced saving should take that long; if anything it tells me you are probably loading way too much data in your Form to be handy (yielding long Form load times as well then).
A paging system (such as the CP forums themselves) might be more appropriate.
|
|
|
|
|
the other thng you may want to do.. And it might be conceptually easier is to
thread[0] : run Main -> all your saving loading etc..
thread[1] : run Form
thread[2] : other(background worker from Form)
to the user this will look like the application "Quit" however it will continue running in the background until it completes it's task.
|
|
|
|
|
It sounds like your thread is accessing the child form - maybe the progress control? - after the form has closed and been disposed.
Are you sure you want to cancel the thread when the form closes? Wouldn't it be better to allow the thread to complete? If you do want to cancel, you will have to add cancel logic to your thread method. Don't use Thread.Abort() ! If you want to let the thread complete, you will have to make it check if the form still exists before trying to access it.
BTW, Thread.IsBackground just means the thread will be killed when the main form is closed instead of keeping the process running until it has completed.
Nick
----------------------------------
Be excellent to each other
|
|
|
|
|
I used Thread.IsBackground because I was thinking I wanted it to quit when that form was closed. It is a inventory application so if a user started saving a file that was 10MB and it was taking to long I was thinking I wanted to let them close it and I was trying to make it stop (not upload the file).
But I think I might go with the suggestion above and just refuse closing the form until it has completed. Just for my information if I wanted to do something like above, would it be easy to do? Stopping a cmd.ExecuteNonQuery(); that is uploading a file?
|
|
|
|
|
There is an IDbCommand.Cancel() method, but I've never used it.
Nick
----------------------------------
Be excellent to each other
|
|
|
|
|
I would do things a bit differently, in three ways...
1) Use a BackgroundWorker instead of a Thread (System.ComponentModel namespace). This can be set to raise ProgressChanged events on the GUI thread, so you don't have to deal with the invokes. It also has an IsBusy property that you can use when deciding whether to allow the user to close the form.
2a) Declare the worker at the form/window level, so you can kill it when the form is closing (Or not allow the close if it's running)
OR
2b) Create an IsBusy property on the form. Set it to true just before starting the thread/worker, and set it back to false when the worker completes (BackgroundWorkers have a RunWorkerCompleted event you can catch)
3) Instead of having the thread access form-level variables (Particularly if you use (2b) above, create a quick struct with the two strings and bool in it, and pass that as an argument to the thread/worker. It's preferable to have background threads be as self-contained as possible.
|
|
|
|
|
Ahh that is a great idea to.. using a struct.. so you are talking about this:
struct UploadFileInfo
{
public string TAG;
public string fileName;
public string safeFileName;
public bool fileMR;
}
private void saveFiles(object information)
{
UploadFileInfo fileInfo = (UploadFileInfo)information;
.............code.............
}
Thread fileThread = new Thread(new ParameterizedThreadStart(saveFiles));
fileThread.IsBackground = true;
fileThread.Start(upload);
|
|
|
|
|
|
Now that I think about it... the reason I am not using a background worker is because there are two different spots to save data. One is for normal attachments and the other is for M&R part of our inventory. I worked it into the same method to work both.
If I use a background worker I would hvae to create two seperate ones and create two seperate DoWork's. I think I am going to stick with the threading but not let the user close until their upload is complete.
Thanks again!
|
|
|
|
|
Jacob Dixon wrote: the reason I am not using a background worker is because there are two different spots to save data
there is a flaw in that logic. Just make a method (or even a separate class) that does it all (in any way you see fit, Thread, BGW, whatever), then call it from anywhere you like, and take precautions for it not to run twice at the same time.
|
|
|
|
|
In the form's Closing event handler I'd check on the thread and pop up a message that it can't close yet.
You could also have the thread disable the form's close buttons[^] until it completes, but that may be a bit heavy-handed.
|
|
|
|
|
I wanted to look up how the function TimeDiff is implemented exactly (a function to calculate how many hours there are between a start time and an end time) and found this. I'm not that experienced yet, so tell me... is there a way in which this might make sense or is this simply redundant and... well, stupid?
public static XDateTime TimeDiff(XDateTime d1, XDateTime d2)
{
#region Local Variables
#endregion
#region Actions
return APTimeDiff(d1, d2);
#endregion
}
APTimeDiff returns the same type and does the actual calculation (and formatting). Why put it into another function?
|
|
|
|
|
Well, its just a guess but one reason for doing this might be to avoid changing the public API of the class. So, once upon a time method TimeDiff did something different. Then it was decided to use APTimeDiff instead (maybe it's more efficient, maybe it handles some corner condition more effectively). But it might be that this is a library class which is used in a number of places, and for whatever reason we can't go through and change all the calls to TimeDiff to call APTimeDiff instead. So, just leave the old method in place but change the internal implementation. Any new code should now be using APTimeDiff, but old code that uses TimeDiff will continue to function as is and does not need changed.
It's a fairly common thing to do as part of a Refactoring process. Eventually, the aim would be to work through and replace all calls to TimeDiff with APTimeDiff.
|
|
|
|
|
It looks like half completed code, or post-refactored code which has not been cleaned up.
In the former case, the completion of that method would perhaps put some validation in to check the inputs.
In the latter case, the method may have been left as there are many places around the application calling TimeDiff and changing them all to APTimeDiff may be too time consuming at the current time.
|
|
|
|
|
Thanks guys, that makes sense. It is source code that has been passed on for many versions and many years, so it may very well be a change that has been made without cleaning the older parts up properly.
|
|
|
|
|
Is there something that I'm not understanding properly here? I've written a DLL that our customers can use to connect to our server and exchange certain info. On the one hand it makes it easier for them, not having to code a bunch of comms stuff and on the other hand it hides our exact protocol from them (or so I hoped).
When I use the DLL in my own test application and I set breakpoints in my code I can step right into the functions in the DLL. Surely it should be possible to avoid this? Any help please?
|
|
|
|
|
You can step into the DLL in your test application because it is debug compiled. You will provide a release compiled DLL to your customer. If you want to "hide" your code even more you can run it through an obfuscator, for instance dotfuscator.
|
|
|
|
|
Thanks for the reply. I will have a look at Dotfuscator.
Just to be clear, my DLL was indeed compiled with RELEASE configuration, not DEBUG. Now I'm not even sure what the difference is anymore as I always thought this was one of the key differences between the two configurations.
|
|
|
|
|
You can never entirely avoid leaking the code though. You can not depend on your code being secret. The CPU can still run the code, and everything the CPU does to be able to read the code is something the user could do as well. There is no solution.
A protocol is even harder to hide since they could capture their own internet traffic and analyse it.
But you can make it harder for your customer to read your code by using an obfuscator as indicated by Calla
|
|
|
|
|
It makes sense, thanks. I can understand that you can never prevent someone from decompiling your code but I would like for it to be a bit harder than this! It looks like Dotfuscator might do the job, I'll look into it, thanks.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
