1

I'm performing an async operation for an upload using Starksoft.Net.Ftp.

Looks like that:

    public void UploadFile(string filePath, string packageVersion)
    {
        _uploadFtpClient= new FtpClient(Host, Port, FtpSecurityProtocol.None)
        {
            DataTransferMode = UsePassiveMode ? TransferMode.Passive : TransferMode.Active,
            FileTransferType = TransferType.Binary,
        };
        _uploadFtpClient.TransferProgress += TransferProgressChangedEventHandler;
        _uploadFtpClient.PutFileAsyncCompleted += UploadFinished;
        _uploadFtpClient.Open(Username, Password);
        _uploadFtpClient.ChangeDirectoryMultiPath(Directory);
        _uploadFtpClient.MakeDirectory(newDirectory);
        _uploadFtpClient.ChangeDirectory(newDirectory);
        _uploadFtpClient.PutFileAsync(filePath, FileAction.Create);
        _uploadResetEvent.WaitOne();
        _uploadFtpClient.Close();
    }

    private void UploadFinished(object sender, PutFileAsyncCompletedEventArgs e)
    {
        if (e.Error != null)
        {
            if (e.Error.InnerException != null)
                UploadException = e.Error.InnerException;
        }
        _uploadResetEvent.Set();
    }

As you can see, there is a ManualResetEvent in there, which is declared as private variable on top of the class:

private ManualResetEvent _uploadResetEvent = new ManualResetEvent(false);

Well, the sense is just that it should wait for the upload to complete, but it must be async for reporting progress, that's all.

Now, this just works fine. I have a second method that should cancel the upload, if wished.

public void Cancel()
{
    _uploadFtpClient.CancelAsync();
}

When the upload is cancelled a directory on the server also must be deleted. I have a method for this, too:

    public void DeleteDirectory(string directoryName)
    {
        _uploadResetEvent.Set(); // As the finished event of the upload is not called when cancelling, I need to set the ResetEvent manually here.

        if (!_hasAlreadyFixedStrings)
            FixProperties();

        var directoryEmptyingClient = new FtpClient(Host, Port, FtpSecurityProtocol.None)
        {
            DataTransferMode = UsePassiveMode ? TransferMode.Passive : TransferMode.Active,
            FileTransferType = TransferType.Binary
        };
        directoryEmptyingClient.Open(Username, Password);
        directoryEmptyingClient.ChangeDirectoryMultiPath(String.Format("/{0}/{1}", Directory, directoryName));
        directoryEmptyingClient.GetDirListAsyncCompleted += DirectoryListingFinished;
        directoryEmptyingClient.GetDirListAsync();
        _directoryFilesListingResetEvent.WaitOne(); // Deadlock appears here

        if (_directoryCollection != null)
        {
            foreach (FtpItem directoryItem in _directoryCollection)
            {
                directoryEmptyingClient.DeleteFile(directoryItem.Name);
            }
        }
        directoryEmptyingClient.Close();

        var directoryDeletingClient = new FtpClient(Host, Port, FtpSecurityProtocol.None)
        {
            DataTransferMode = UsePassiveMode ? TransferMode.Passive : TransferMode.Active,
            FileTransferType = TransferType.Binary
        };
        directoryDeletingClient.Open(Username, Password);
        directoryDeletingClient.ChangeDirectoryMultiPath(Directory);
        directoryDeletingClient.DeleteDirectory(directoryName);
        directoryDeletingClient.Close();
    }

    private void DirectoryListingFinished(object sender, GetDirListAsyncCompletedEventArgs e)
    {
        _directoryCollection = e.DirectoryListingResult;
        _directoryFilesListingResetEvent.Set();
    }

As the finished event of the upload is not called when cancelling, I need to set the ResetEvent manually in the DeleteDirectory-method.

Now, what am I doing here: I first list all files in the directory in order to delete them, as a filled folder can't be deleted.

This method GetDirListAsync is also async which means I need another ManualResetEvent as I don't want the form to freeze.

This ResetEvent is _directoryFilesListingResetEvent. It is declared like the _uploadResetEvent above.

Now, the problem is, it goes to the WaitOne-call of the _directoryFilesListingResetEvent and then it stucks. A deadlock appears and the form freezes. (I've also marked it in the code)

Why is that? I tried to move the call of _uploadResetEvent.Set(), but it doesn't change. Does anyone see the problem?

When I try to call the DeleteDirectory-method alone without any upload, it works as well. I think the problem is that both ResetEvents use the same resource or something and overlap themselves, I don't know.

Thanks for your help.

Dominic B.
  • 1,897
  • 1
  • 16
  • 31
  • Could the upload put a lock on the directory you are trying to dele? – jaywayco Oct 11 '14 at 18:38
  • Good point, that could be, as I create it there, but shouldn't the "CancelAsync" manage that? – Dominic B. Oct 11 '14 at 18:46
  • Does cancelasync trigger the uploadfinished event? If not that upload client will not get closed and could be hanging on to resources – jaywayco Oct 11 '14 at 18:48
  • Nope, it is not triggered, but the problem continues as the deadlock still appears, even when forcing to close the connection. – Dominic B. Oct 11 '14 at 19:03
  • As i understand from the discussion following code in the private void DirectoryListingFinished _uploadResetEvent.Set(); will do the job, as here the issue seems to be @ _uploadResetEvent.WaitOne();, not at other WaitOne – Mrinal Kamboj Oct 11 '14 at 19:06
  • Inserted the _uploadResetEvent.Set(); there, but deadlock is still there. – Dominic B. Oct 11 '14 at 19:10
  • 1
    @Alireza Thanks for your feedback. No, the problem is still there an the deadlock just doesn't want to give up... – Dominic B. Oct 16 '14 at 05:44
  • 1
    Just wanted to check. Will give it a try tonight :) – Alireza Oct 16 '14 at 11:58

2 Answers2

3

You are not using this library correctly. The MREs you added cause deadlock. That started with _uploadResetEvent.WaitOne(), blocking the UI thread. This is normally illegal, the CLR ensures that your UI does not go completely dead by pumping a message loop itself. That makes it look like it is still alive, it still repaints for example. A rough equivalent of DoEvents(), although not nearly as dangerous.

But the biggest problem with it is that it will not allow your PutFileAsyncCompleted event handler to run, the underlying async worker is a plain BackgroundWorker. It fires its events on the same thread that started it, which is very nice. But it cannot call its RunWorkerCompleted event handler until the UI thread goes idle. Which is not nice, the thread is stuck in the WaitOne() call. Exact same story for what you are debugging now, your GetDirListAsyncCompleted event handler cannot run for the same reason. So it just freezes there without being able to make progress.

So eliminate _uploadResetEvent completely, rely on your UploadFinished() method instead. You can find out if it was canceled from the e.Cancelled property. Only then do you start the code to delete the directory. Follow the same pattern, using the corresponding XxxAsyncCompleted event to decide what to do next. No need for MREs at all.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • Ah ok, so the AsyncCompletedEventArgs are the ones I need to give attention. I will try it, thanks. – Dominic B. Oct 17 '14 at 20:26
  • So: I kept the MRE for the upload because I need to wait for the method to finish. But now I moved the deleting to the UploadFinished-handler in a query where the Cancelled-property is checked. Thanks, it works fine, now! – Dominic B. Oct 18 '14 at 20:40
1

Looking at the source, it appears FtpClient uses a BackgroundWorker to perform asynchronous operations. That means its completion event will be posted to whatever SynchronizationContext was set at the time the worker was created. I'll bet the completion of CancelAsync pushes you back onto the UI thread, which blocks when you call WaitOne on the directory list reset event. The GetDirListAsyncCompleted event gets posted to UI message loop, but since the UI thread is blocked, it will never run, and the reset event will never be set.

BOOM! Deadlock.

Mike Strobel
  • 25,075
  • 57
  • 69
  • Thanks for the explanation. Is there anything I could try to fix that? – Dominic B. Oct 17 '14 at 20:02
  • You might try getting rid of the reset event for the directory list operation, and kick off the directory deleting code directly from the `GetDirListAsyncCompleted` handler. – Mike Strobel Oct 17 '14 at 20:08
  • Also, as a rule, any time you have a thread waiting indefinitely for a signal, you should do everything possible to ensure the signal gets sent (even if the operation you're waiting on faults). I'd recommend calling `Set()` in a `finally` block just in case anything leading up to the signal throws. Oh, and also never block the UI thread :D. – Mike Strobel Oct 17 '14 at 20:10