Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: SendFileAsync Dispose()s the provided Stream #2810

Open
3 tasks done
HLSSLenda opened this issue Dec 16, 2023 · 2 comments
Open
3 tasks done

[Bug]: SendFileAsync Dispose()s the provided Stream #2810

HLSSLenda opened this issue Dec 16, 2023 · 2 comments
Assignees
Labels

Comments

@HLSSLenda
Copy link

Check The Docs

  • I double checked the docs and couldn't find any useful information.

Verify Issue Source

  • I verified the issue was caused by Discord.Net.

Check your intents

  • I double checked that I have the required intents.

Description

If I open a Stream and I call RespondWithFileAsync or SendFileAsync, the Stream gets disposed after Discord.NET sends the message.
If the caller allocates or gets somehow a Stream, the caller should be responsible for calling Dispose on the stream, not Discord.NET itself.

FileAsync wraps the stream under a FileAttachment inside a using block, disposing it and the Stream itself at the end of the method (not familiarized enough with Discord.NET code to do a pull request though)

Version

3.13.0

Working Version

No response

Logs

System.Net.Http.HttpRequestException: Error while copying content to a stream.
 ---> System.ObjectDisposedException: Cannot access a closed Stream.

Sample

// inside a command
const string NAME = "i_dont_have_creativity_for_a_png_name.png";
using Stream myImage = File.OpenRead(NAME);
await RespondWithFileAsync(myImage, NAME, "Look at my image!");
await FollowupWithFileAsync(myImage, NAME, "LOOK AGAIN, YOU'RE NOT LOOKING ENOUGH"); // ObjectDisposedException here

Packages

null

Environment

  • OS: Debian GNU/Linux 12
  • Architecture: x64
  • SDK: 8.0.100
@HLSSLenda HLSSLenda added the bug label Dec 16, 2023
@DutchSlav
Copy link

I'm not completely sure, but your code

using Stream myImage = File.OpenRead(NAME);

is the problem. "using" indicates that it should dispose of itself after the first use.
Hope this helps!

@BobVul
Copy link
Contributor

BobVul commented Dec 27, 2023

"using" indicates that it should dispose of itself after the first use.

Nope. using disposes when it exits the scope.

{
    using var foo = Bar();
    // use foo
    // use foo again?
}

is equivalent to

using (var foo = Bar())
{
    // use foo
    // use foo again?
}

is equivalent to

var foo = Bar();
try
{
    // use foo
    // use foo again?
}
finally
{
    foo?.Dispose();
}

You may use the disposable object as many times as you wish within the using scope. Of course, many stream types are not seekable, so using it multiple times often does not make sense. Since you know you have a FileStream that is seekable, you'd normally be expected to seek back to the beginning before reuse. That said Discord.Net does internally try to seek back to 0 if possible.


HLSSLenda is correct that the culprit is

using (var file = new FileAttachment(fileStream, fileName))
, which wraps the provided stream inside a FileAttachment with its own using. You could probably get around this by constructing your own FileAttachment and passing it in via RespondWithFileAsync(FileAttachment attachment, ..., but this also bypasses the seek so you must also attachment.Stream.Position = 0 in between uses of it unless you actually expect the stream to provide fresh data.

And yea the library probably shouldn't be disposing streams it receives but does not own. Perhaps FileAttachment could be extended to work in the same way StreamWriter etc. do, with a leaveOpen param that indicates it should not dispose the underlying stream when it is disposed itself? Then RespondWithFileAsync could pass that when it constructs the attachment wrapper.

@Misha-133 Misha-133 self-assigned this Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants