ravibpatel / AutoUpdater.NET

AutoUpdater.NET is a class library that allows .NET developers to easily add auto update functionality to their classic desktop application projects.
MIT License
3.05k stars 770 forks source link

Once download complete , autoupdater not close the running programm first. the update failed #11

Closed ghost closed 7 years ago

ghost commented 7 years ago

Once download complete , autoupdater not close the running programm first. the update failed

The autoupdater just restat running programm, and it still old version , not update to latest version.

so why not close running program, then update, then start new program.

ravibpatel commented 7 years ago

If you see the source code it just does that. Which type of application are you developing (WPF, Winforms, Console etc.)?

ghost commented 7 years ago

winforms. https://github.com/netroby/border-less-browser/blob/master/BorderLessBrowser/BrowserForm.cs

I am sure it does not close program, the updater hang up .

ghost commented 7 years ago

https://github.com/ravibpatel/AutoUpdater.NET/blob/master/AutoUpdater.NET/DownloadUpdateDialog.cs#L83

can you please provide an api to force exit the parent process/application

ghost commented 7 years ago

for Example.

Autoupdate.OnDownloadComplete(()=>Application.Exit())
ghost commented 7 years ago

https://github.com/ravibpatel/AutoUpdater.NET/blob/master/AutoUpdater.NET/DownloadUpdateDialog.cs#L66

The problem is this . you really should close the running parent programm, then you can run zip extract . or the zip extract would not override the running files

ravibpatel commented 7 years ago

https://github.com/ravibpatel/AutoUpdater.NET/blob/master/AutoUpdater.NET/DownloadUpdateDialog.cs#L74-L90

If you see above those lines are dedicated to doing just that. Close currently running application.

ghost commented 7 years ago

sorry. it not works. but i found other solution

On Thu, May 4, 2017 at 8:11 PM +0800, "Ravi Patel" notifications@github.com wrote:

https://github.com/ravibpatel/AutoUpdater.NET/blob/master/AutoUpdater.NET/DownloadUpdateDialog.cs#L74-L90

If you see above those lines are dedicated to doing just that. Close currently running application.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

guillaumejay commented 7 years ago

I have the same problem with a WPF app (using currently click once, but autoupdater is only used when not clickonce-launched), and if I don't kill manually the application, autoupdater is not able to kill it.

I'm wondering if there's an event I should use to kill my application gracefully by itself.

(I think I can send you my application if needed to check what is happening..)

ravibpatel commented 7 years ago

@guillaumejay Yeah, if you can provide me with the application I can see what is the issue.

guillaumejay commented 7 years ago

Would it be relevant if I only send (where) you the relevant exe (and its dll) ?

We worked a bit and found two things :

ravibpatel commented 7 years ago

@guillaumejay As long as I can test AutoUpdate part it's completely fine if you send only relevant exe and DLL.

guillaumejay commented 7 years ago

I sent you a message using uservoice. Thanks for any help you can bring. I'll try to take a look on my side, I'm not sure my app is well optimized.

JasonXtreme commented 7 years ago

I want to confirm this issue, as I am experiencing it as well. WPF application, .net 4.0 Client profile - If I manage to click the update button before the application starts, all runs well, once my Initialize function goes through, the update fails and the old version is restarted.

ravibpatel commented 7 years ago

@JasonXtreme Do you use CefSharp?

JasonXtreme commented 7 years ago

@ravibpatel I do not. I downloaded the source code, found the issue was within ZipExtractor - where you check for (speaking from memory, on my phone) Process.StartInfo.FileName.Equals(args[2]), the FileName was consistently empty (for all 170 processes). Changing StartInfo to MainModule fixed it for me.

ravibpatel commented 7 years ago

Nice find. Just read the MSDN and found out the reason for this. My bad for not reading the full doc.

"If you did not use the Start method to start a process, the StartInfo property does not reflect the parameters used to start the process. For example, if you use GetProcesses to get an array of processes running on the computer, the StartInfo property of each Process does not contain the original file name or arguments used to start the process."

Can you open a pull request with that particular change so I can add you as a contributor?

guillaumejay commented 7 years ago

Great, I hope it will solve my problem.

On Fri, Jun 9, 2017 at 6:11 PM, Ravi Patel notifications@github.com wrote:

Nice find. Just read the MSDN and found out the reason for this. My bad for not reading the full doc.

"If you did not use the Start method to start a process, the StartInfo property does not reflect the parameters used to start the process. For example, if you use GetProcesses to get an array of processes running on the computer, the StartInfo property of each Process does not contain the original file name or arguments used to start the process."

Can you open a pull request with that particular change so I can add you as a contributor?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ravibpatel/AutoUpdater.NET/issues/11#issuecomment-307432140, or mute the thread https://github.com/notifications/unsubscribe-auth/AE-uVmoNJ-XEpUTPIjAsfoF5uIlmKsf1ks5sCW62gaJpZM4NO540 .

JasonXtreme commented 7 years ago

Gladly, but it won't be before monday. Cheers!

On Jun 9, 2017 18:11, "Ravi Patel" notifications@github.com wrote:

Nice find. Just read the MSDN and found out the reason for this. My bad for not reading the full doc.

"If you did not use the Start method to start a process, the StartInfo property does not reflect the parameters used to start the process. For example, if you use GetProcesses to get an array of processes running on the computer, the StartInfo property of each Process does not contain the original file name or arguments used to start the process."

Can you open a pull request with that particular change so I can add you as a contributor?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ravibpatel/AutoUpdater.NET/issues/11#issuecomment-307432140, or mute the thread https://github.com/notifications/unsubscribe-auth/ABlXXUc2BoVhmQkXt29iL-aZcwpLJmysks5sCW62gaJpZM4NO540 .

ravibpatel commented 7 years ago

@guillaumejay I afraid it won't fix your problem. I created a minimal application using CefSharp and Environment.Exit() can't kill it. I am working on a workaround. I will email you a fix when I have it.

guillaumejay commented 7 years ago

Did you manually call cef.Shutdown too ?

On Sat, Jun 10, 2017 at 7:12 AM, Ravi Patel notifications@github.com wrote:

@guillaumejay https://github.com/guillaumejay I afraid it won't fix your problem. I created a minimal application using CefSharp and Environment.Exit() can't kill it. I am working on a workaround. I will email you a fix when I have it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ravibpatel/AutoUpdater.NET/issues/11#issuecomment-307543344, or mute the thread https://github.com/notifications/unsubscribe-auth/AE-uVr_Fjf_gJqj5p_L80LlmEeE7tTU2ks5sCiWrgaJpZM4NO540 .

ravibpatel commented 7 years ago

@guillaumejay No, but when I tried with Event Application.Current.Shutdown() can kill the application.

stebla27 commented 7 years ago

I have the same problem. In my WPF application sometimes update works and sometimes not. Seems to be a timing issue. When update fails the application restarts with the same version as before and the update dialog is shown again. Starting update a second time is sometimes working, sometimes the updater hangs forever. Is there any workaround solution?

ravibpatel commented 7 years ago

@stebla27 I got the fix. Just download the Beta from here and try it out. You need to handle ApplicationExitEvent though. I am still working on it so you don't have to do it in future.

AutoUpdater.Start("http://rbsoft.org/updates/AutoUpdaterTest.xml");

AutoUpdater.ApplicationExitEvent += delegate
{
  Application.Current.Shutdown();
};
stebla27 commented 7 years ago

@ravibpatel Thanks a lot for fast help. My first tests were positive! The possibility for an ApplicationExitEvent is not so bad. 👍 But I have a feature request for this: It would be nice if i could cancel update in this event by passing a custom error/info text. Or maybe it is better to specify another Event BeforeApplicationUpdate. My use case is as follows: The Auto Update is started when application opens. The user first ignors the update dialog and then starts a long pending operation which should not be aborted. If the user now starts the update I would like to ask him if the current operation should be aborted to start the update or to remind him later. If I have to abort the operation I can use the ApplicationExitEvent.

ravibpatel commented 7 years ago

@stebla27 There is event already available for this purpose. See Handling updates manually section. You can do whatever you want to do before downloading and installing the update in this event. ApplicationExitEvent is not so bad but the idea of adding an another line for it to work bothers me. It should me nice if the developer can add AutoUpdate functionality using just one line.

stebla27 commented 7 years ago

The problems unfortunately still exists. There are still update loops. The program says, there is an update. Customer clicks on update. Old program starts again and says that there are updates available. It's random.

ravibpatel commented 7 years ago

@stebla27 It was happening with the beta I provided earlier too?

stebla27 commented 7 years ago

@ravibpatel: As it does not happen on all PCs and it is a timing issue I'm not 100% sure. But with the beta I got no customer reports. After upgrading to the official nuget package (including your fix) the behaviour is just like before. Sorry for bad message! ;)

ravibpatel commented 7 years ago

@stebla27 Problem with the beta is if user use Remind later option then it throws an error when ApplicationExitEvent is called.

ravibpatel commented 7 years ago

@stebla27 Which .NET Framework you use and is your application using any third party libraries other than this?

stebla27 commented 7 years ago

@ravibpatel .Net Framework 4.5.2 Third Party components: CommonServiceLocator Costura.Fody FontAwesome.WPF Microsoft.Bcl morelinq Newtonsoft.Json Nito.AsyncEx Ookii.Dialogs Pri.LongPath

ravibpatel commented 7 years ago

@stebla27 Where you call the Start method (constructor of MainWindow)?

stebla27 commented 7 years ago

@ravibpatel Yes in constructor of MainWindow.

stebla27 commented 7 years ago

@ravibpatel Regarding the "Remind later" option A customer also reported an exception in UpdateForm.Designer.cs when he clicks on "Remind me later" and the update notice time has expired.

ravibpatel commented 7 years ago

@stebla27 Can't reproduce the Remind Later exception. Did you get the stack trace for the error? Can you share me the location of your XML file? If you can't do it here, you can share it on http://rbsoft.uservoice.com.

stebla27 commented 7 years ago

@ravibpatel Unfortunately I have no stack trace. Only the information that exception occurred in UpdateForm.Designer.cs. I can't provide you the location of the XML file as it is stored on an internal server and only accessible from an internal webserver.

ravibpatel commented 7 years ago

@stebla27 Can you try the beta from below to see if it fixes your problem?

https://www.dropbox.com/s/xg2543cmlcff04a/AutoUpdater.NET.zip?dl=0

stebla27 commented 7 years ago

@ravibpatel: The problem is I can't reproduce it on my machine. I've only see it on customer platforms and here I can't really test. I checked the code and I have two questions: In Exit method: foreach (var process in Process.GetProcessesByName(currentProcess.ProcessName)) { if (process.Id != currentProcess.Id) { process.Kill(); } }

        if (IsWinFormsApplication)
        {
            Application.Exit();
        }
    #if NETWPF
        else if (System.Windows.Application.Current != null)
        {
            System.Windows.Application.Current.Dispatcher.BeginInvoke(new Action(() => System.Windows.Application.Current.Shutdown()));
        }
    #endif
        else
        {
            Environment.Exit(0);
        }

Should there an additional check if application is already closed? For example after the BeginInvoke. Or after process.Kill? Furthermore I'm not sure if the exe can be overwritten directly after closing the process. Sometimes Windows blocks write access for some milliseconds and an I/O exception occurs. Have you implemented a retry behaviour for this?

ravibpatel commented 7 years ago

@stebla27 There is an additional check present in ZipExtractor that wait for the process to completely exit before extracting anything from downloaded archive. As you can see here. There is no retry behaviour currently present now but I can write one.

stebla27 commented 7 years ago

@ravibpatel : Ok, waiting for the process to exit is OK. You don't need a retry behaviour here. But when you overwrite the file on extracting (don't know where in your code) it could be that windows hasn't released the file handle yet and you will get an I/O Exception. In this case you have to wait some time and retry it again. I had the same problem in my application and it is timing dependent when you can write a file after last file access. So even if windows says the process has exited it could be that associated exe can't be overwritten for some miliseconds. It seems that on slower machines (e.g. no SSD) the error happens more often.

Example: do { //try to load the file //the file could be not yet ready, so we have to do a retry loop here. try { file = File.OpenText(Path); retryCount = 0; } catch (IOException e) { if (File.Exists(Path)) { if (retryCount == 0) retryCount = 5;

                        Thread.Sleep(1000);
                    }
                    else
                        throw e;
                }
            }
            while (retryCount-- > 0);
LZong-tw commented 6 years ago

I tested AutoUpdater on Windows Embedded POSReady 2009 Version 2.0 SP3(could be seen as XP) with .NET 4.0, and the issue still exists. Although it could success after I tried clicking update button for a few times(maybe 3~5 times), it still can't be acceptable to use. But when I added

AutoUpdater.ApplicationExitEvent += delegate
{
    Application.Exit();
};

inside the Main UI and after AutoUpdater.Start(updateXMLpath);, the problem seems to be solved pretty well.