AutomaticUpdater race conditions [bugfix submission]

I found thread race condition bugs when creating-->running--> disposing--> recreating --> running an instance of AutomaticUpdaterBackend.

I found this problem when trying to workaround an update timeout bug that causes the AutomaticUpdater to have a corrupt state (not able to update)

my bugfix allows creating-->running--> disposing--> recreating --> running an instance of AutomaticUpdaterBackend.

my fix is simply to lock the sections of UpdateHelper and PipeClient that cause issues (Because parts of these classes run on different threads) and the other threads crash when the object is disposed.

Here are my fixes. I have verified it works in my dev environement (including downloading and installing an update), but i haven't done a broad usage testing yet.

UpdateHelper.cs: http://pastebin.com/cqsrRKz4PipeClient.cs: http://pastebin.com/75LRatx7

FYI, my UpdateHelper.cs also comments out the call to Application.DoEvents(); in RecreateBackgroundWorker()

IMO that is a bug, and i needed to comment that out for my app to behave correctly. (Message Pumping deep inside of your app is not a good idea!)

sorry one more thing, i got the code around 1 month ago, so any bugfixes since then wouldn't be included.

Looking over this patches briefly I can see they fix a number of bugs. I'll look them over a little closer and we'll thoroughly test them before including them in our SVN tree.

Thanks for doing this.

FYI, my fix introduces a deadlock: PipeClient.Read(), if(Connected) causes it to lock PipeClient and attempt to lock UpdateHelper

another thread could be locking UpdateHelper and attempt to lock PipeClient, resulting in deadlock. (this can happen, for example, by UpdateHelper.RetrySend() calling PipeClient.SendMessage()

the fix for this is to make sure pipeClient never calls anything that can end up locking UpdateHelper.Thankfully, this only means changing one function, the PipeClient.Read() which I already described, plus the MessageReceived section of the same .Read() method

the fixed code can be found at http://pastebin.com/b3MzQbSM

sorry i pasted the wrong version (bug on line 252)

here's the correct version of the fixed PipeClient.cs: http://pastebin.com/cgLrUd79

Thanks for the update. We're reviewing it now.

fyi i've been running this in my dogfood environment (about 10 machines) and so far no issues.

Thanks novaleaf -- we've included the fixes in the trunk.

Reply
Discard post