ume05rw / SharpCifs.Std

SharpCifs.Std is a port from SharpCifs to .NET Standard. Original SharpCifs: https://github.com/zinkpad/SharpCifs
GNU Lesser General Public License v2.1
86 stars 34 forks source link

WrappedSystemStream should override Dispose() to dispose internal stream. #19

Open haru666 opened 6 years ago

haru666 commented 6 years ago

Summary : InputStream implicitly cast itself to System.IO.Stream. This indicates the given back stream should free its internal resource when disposing, but WrappedSystemStream not override Dispose method nor dispose it's internal stream after using.


まず初めに、このライブラリを共有して頂きありがとうございます。 日本人の方のようですので日本語でIssueを上げさせて頂きます。

InputStreamStreamへの暗黙的な型変換を用意しています。 しかしWrappedSystemStreamDispose時にInputStreamDisposeをコールしないため、これに起因して下記のようなコードではInputStreamが解放されなくなってしまいます。 暗黙的に型変換できるということは、通常はアップキャスト(正確にはこの場合違いますが)してから責任を移譲した場合、Stream型として扱っても内部リソースを開放できる必要があると思います。

Task<Stream> OpenReadStream()
{
    var smbFile = new SmbFile(...);
    return await smbFile.GetInputStreamAsync();
}

using (var stream = await OpenReadStream())
{
    // do something here 
} // this will not dispose InputStream

上記はC#としては自然なコードだと思っています。 C#の他のリソースとの連携を考えると、Stream型の方が都合がよく、可読性も向上します。 しかし、現在のコードだとInputStreamが開いたままになってしまうため、戻り値をInputStreamにせざるを得ません。

意図的にCloseについてもoverrideせずキャストされたStreamCloseした時のInputStream解放を避けているようですが、何か理由があるのでしょうか?

ume05rw commented 6 years ago

お問い合わせありがとうございます。

母語で読み書き出来るというのは、思ったよりもずっと良いものですね。 皆さまからのご指摘はいつも有難く勉強させて頂いておりますが、 母語での指摘による伝達効率がこれほど違うものか、と驚いております。

Google翻訳のインチキ英語で恥を晒さずに済みます(笑

さてご指摘の件ですが、恐らくSharpenライブラリ内部の実装を指しておられるものと思います。 ご存知かもしれませんが、SharpenはJava<-->C#の相互変換を目指したプロジェクトです。 Javaのコードを機械的に変換して動かす都合上、様々な問題点を内包していることを、私も認識しております。 両者の違いが大きくなってしまったためでしょうか、現在はほぼメンテナンスされていない状況のようです。

しかし現在このライブラリには、これとは別の大きな問題点があります。 昨年のWannaCryウイルスの影響で、SMB1.0が非推奨化されました。 Sharpcifs.stdは本家JCIFS同様、SMB1.0までの実装につき、致命的に陳腐化してしまいました。

せっかくご指摘を頂いたところを申し訳ないのですが、今後のメンテナンスについては あまりご期待に沿えないように思います。どうかご容赦くださいませ。