Skip to content

Add logic to make the receipt of an UploadFailed message from a remote client fail the associated download #877

@jpdillingham

Description

@jpdillingham

I encountered a scenario in slskd where the remote client (another copy of slskd, but it can happen with any) times out waiting for a response to the request to start the transfer. This fails the upload on the sending side and causes an UploadFailed/code 46 to be sent.

Currently this message only raises an event; it doesn't interact with the download logic at all.

Add logic to cause this message to fail the associated download, if one is active and waiting. I'm thinking:

Add the following to TransferInternal:

        /// <summary>
        ///     Gets the task completion source used to end the transfer if/when the remote client reports that it has failed or been rejected.
        /// </summary>
        public TaskCompletionSource<bool> RemoteTaskCompletionSource { get; } = new TaskCompletionSource<bool>();

Add the following to the DownloadFailed binding in SoulseekClient:

            PeerMessageHandler.DownloadFailed += (sender, e) =>
            {
                try
                {
                    // assumes controls are in place (as they were at the time of this implementation) to prevent
                    // duplicate transfers of the same file from the same user
                    DownloadDictionary.Values
                        .FirstOrDefault(d => d.Username == e.Username && d.Filename == e.Filename)
                        ?.RemoteTaskCompletionSource.TrySetException(new TransferException("Transfer reported as failed by the remote client"));
                }
                finally
                {
                    DownloadFailed?.Invoke(this, e);
                }
            };

And finally adjust the logic in DownloadToStreamAsync like so:

                var firstTask = await Task.WhenAny(disconnectedTask, download.RemoteTaskCompletionSource.Task, readTask).ConfigureAwait(false);

                // cancel the losing task
                linkedCancellationTokenSource.Cancel();

                if (firstTask == download.RemoteTaskCompletionSource.Task)
                {
                    // we should only use this to signal an abnormal (unsuccessful) end of the transfer, which
                    // should set an exception for the task. awaiting it will raise it and enter a catch
                    await download.RemoteTaskCompletionSource.Task.ConfigureAwait(false);
                }
                else if (firstTask == disconnectedTask)
                {
                    // this is guaranteed to throw; we control the TCS and we're calling SetException() above
                    await disconnectedTask.ConfigureAwait(false);
                }

                await readTask.ConfigureAwait(false);

With this change the download will end if:

  • All data is read from the connection (readTask completes)
  • The connection is disconnected (disconnectedTask completes)
  • The remote client reports that the transfer failed (RemoteTaskCompletionSource completes)

Whichever comes first.

Some thought needs to go into what exception should be thrown, etc., but the plan presented here should be dev ready.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions