Ticket #38036

CTexture等でのメモリリーク

Open Date: 2018-03-12 23:17 Last Update: 2020-01-20 00:49

Reporter:
Owner:
Type:
Status:
Closed
Component:
Priority:
5 - Medium
Severity:
5 - Medium
Resolution:
Fixed
File:
None
Vote
Score: 0
No votes
0.0% (0/0)
0.0% (0/0)

Details

匿名希望さんからのタレコミより。

CTexture()でIDisoposable I/Fの実装が正しくなく、メモリリークしているのでは?とのご指摘。

Ticket History (3/14 Histories)

2018-03-12 23:17 Updated by: yyagi
  • New Ticket "CTexture等でのメモリリーク" created
2018-03-12 23:37 Updated by: yyagi
  • Details Updated
  • Resolution Update from None to Fixed
Comment

Rev de3d250224baf0e2fb39d7dae70f7a786d28b605 で、CTexture(), CPrivateFont(), CPrivateFastFont()のIDisposaable I/Fの実装を修正済み。

以下余談です。

CTexture()などでファイナライザの実装をしていなかったのは事実ですが、私はこの手の大きなデータは普通手動で開放(Dispose)するものだと思っています。(こういう考えは古い考えでしょうかね...。)

そして、大きなデータはちゃんとDisposeしていますし、またこの辺のクラスはDisposeしていればメモリリークは発生しません。(実際、メモリリークはしていないと思うのですが...)

...なのですが、一方で「ライブラリ(下回り)としてはちゃんとファイナライザも実装すべき」といったご指摘をたくさんいただくのも大変申し訳ないので、今回Rev de3d250224baf0e2fb39d7dae70f7a786d28b605 でIDisposableのデザインパターンに則ってファイナライザ対応いたしました。

また、ダメ押しで、画面遷移の際に強制実施しているGarbage Collectionにて、LOHのCompression(再配置)も実施するようにしました。ここまでやれば、OutOfMemoryはまず発生しないはずです。

(.NETアプリでOutOfMemoryが発生する最大の理由は、LOHに確保されたメモリが再配置されず、空きメモリが断片化することです。興味がおありでしたら、C# GC LOHとかでお調べください。ただ、一方で、LOHの再配置処理はそれなりに重い処理なので、もし画面遷移時間が我慢できなくなるくらい長くなるようでしたら、お知らせください。このダメ押し処理を削除します。)

2018-03-13 01:02 Updated by: from
Comment

お疲れ様っす。

FROMです。

私も一時期誤解してたのですが、

SlimDXとかSharpDXのクラスはネイティブリソースじゃないのでDispose-Finalizeパターンは不要ですよ。

こちらが使ってるのは、あくまで内部にネイティブリソースを隠したマネージドリソースです。

こちらが意識しなくてもあちらでそれを実装してますので、こちらがDisposeを忘れたままFinalizeが呼ばれたとしても、別途あちらがFinalizeされたときに、あちらのネイティブリソースはあちらの責任で解放されます。

# 一方、CSCoreはすべての基本クラスになるComObjectクラスでそれをミスってるので困るんですけどね(涙

DTXMania の C# コードに残ってるソレは、DTXMania を C++/CLI とか C++/CX で組んでた頃のなごりだということにしてください。(汗

といいますか、

Finalizeの中から別のマネージドのDisposeを呼び出すようにされた今回の変更はまずいですよ。

GCからFinalizeが呼び出される順番は保証されていませんので、もしあちらが先にFinalizeされたら、こちらからDisposeを呼び出したときに例外が発生します。

2018-03-14 00:57 Updated by: yyagi
Comment

(すみません、書きかけで一度送信してしまったので、書き込みキャンセルの上、出し直しました)

FROMさん、お忙しいところ、コメントいただきありがとうございます。

こちらが使ってるのは、あくまで内部にネイティブリソースを隠したマネージドリソースです。

はい。まず、そのあたりは理解しているつもりです。(参考文献: 右記など。) https://blogs.msdn.microsoft.com/shozoa/2010/09/07/clr/

こちらが意識しなくてもあちらでそれを実装してますので、こちらがDisposeを忘れたままFinalizeが呼ばれたとしても、別途あちらがFinalizeされたときに、あちらのネイティブリソースはあちらの責任で解放されます。

実は某派生アプリでDisposeがなくてOutOfMemoryに至っていたらしい・・・というのが話の発端なので、(次の段落に続く)

Finalizeの中から別のマネージドのDisposeを呼び出すようにされた今回の変更はまずいですよ。 GCからFinalizeが呼び出される順番は保証されていませんので、もしあちらが先にFinalizeされたら、こちらからDisposeを呼び出したときに例外が発生します。

それは確かにそうですね・・・。それはそうなんですが一方で、話の発端がDispose忘れの救済なので、こちらのファイナライザから向こうのDisposeを呼び出さないわけにはいかない。

こちらのDisposeのなかでtry-catchするしかないかなーなどと思いつつも、あっち(SharpDX)側のdispose/finalizeの実装がどうなっているのかを確かめてみたところ、

というわけで、SharpDX側ではこんな懸念はまるっとお見通しで、ファイナライズでリソース開放しようなどと努々思うな、自力でDisposeしてナンボだという設計だとわかりました(苦笑)。

ここまで長文になりましたが、CTexture()はそういう設計のSharpDXの上にのせるクラスである以上、こちらもファイナライズでの開放はしないという設計にせざるを得ないと思いますので、CTexture()の修正については別途revertしておきます。

一方、CPrivateFont()系については、SharpDX等の外部ライブラリ依存は無いクラスですので、ファイナライザからのDisposeはこのまま生かしておきます。

(Edited, 2018-03-14 00:59 Updated by: yyagi)
2018-03-14 02:15 Updated by: from
Comment

FROMです。

SharpDX側ではこんな懸念はまるっとお見通しで、ファイナライズでリソース開放しようなどと努々思うな、自力でDisposeしてナンボだという設計だとわかりました(苦笑)。

ああ、なるほど。

確かに、_nativePointer はネイティブなのに、ファイナライザでは Release せずに = (void*) 0 してますね。(汗

どっかでこの理由を読んだな、と思い出して探してみたら、どうやらこれっぽいです。

Adding finalizer to DisposeBase

1つのネイティブを複数のマネージドで共有してるせいで、ファイナライザからは Relase しづらいとかなんとか。

SlimDX ではこの .NET と COM の仕様の違いを解決するためにアホみたいに膨れ上がったマッピングテーブルを実装してたと記憶していますが、SharpDX ではどうやってるんでしょうね。

で ちょっと戻って、

話の発端がDispose忘れの救済なので、こちらのファイナライザから向こうのDisposeを呼び出さないわけにはいかない。

Dispose忘れは、システムでは救済できない深いテーマなので放置しましょう 放置しましょう(繰り返した

CPrivateFont()系については、SharpDX等の外部ライブラリ依存は無いクラスですので、ファイナライザからのDisposeはこのまま生かしておきます。

はい。disposeManagedObjects = false での呼び出しなので、ファイナライザからでも問題ないかと。

CTexture については、これが false のときにもマネージドの this.texture を Dispose してるのが問題ですので、true の時だけにするべきでしょうね。

よろしくですー。

2018-03-14 19:58 Updated by: yyagi
Comment

ご紹介いただいたリンクを一通り読ませていただきました。後半でのFinalizeへの罵倒っぷりがたまらないっすね。

ただ、そこで「Finalize時にリソースが残っているようなら、Disposeはしないまでもログは出す」というアイデアをいただきました。これを取り込みましょう。こんな感じで行きます。

protected void Dispose(bool disposeManagedObjects)
{
(略)
	if (disposeManagedObjects)
	{
		// (A) Managed リソースの解放
ここで this.textureの破棄
	}
(略)
}
~CTexture()
{
	// ファイナライザの動作時にtextureのDisposeがされていない場合は、
	// CTextureのDispose漏れと見做して警告をログ出力する
	if (this.texture != null)
	{
		Trace.TraceWarning("CTexture: Disposeされていません(Size=({0}, {1}))", sz画像サイズ.Width, sz画像サイズ.Height );
	}
	this.Dispose(false);
}
2018-03-14 20:15 Updated by: from
Comment

確か EnableTrackingReleaseOnFinalizer は既定で true だったはずですので、 Debug 時のみ EnableReleaseOnFinalizer = true にするようアプリに記述しておけば、同じことを SharpDX の Texture がやってくれますよ。

// ファイナライザの動作時にtextureのDisposeがされていない場合は、

// CTextureのDispose漏れと見做して警告をログ出力する

if (this.texture != null)

{

えーと

ですので

ファイナライザ(~CTexture)からマネージド(this.texture)を参照するのはNGの方向で……(汗

2018-03-14 20:52 Updated by: yyagi
Comment

Debug 時のみ EnableReleaseOnFinalizer = true にするようアプリに記述しておけば、

DebugとReleaseとで、Finalizeのタイミングが違うし・・・とも思いましたが、Dispose漏れのログ出しという見地では大差ないか。

ファイナライザ(~CTexture)からマネージド(this.texture)を参照するのはNGの方向で……(汗

本質的にはマネージドクラス変数(=参照ポインタ)自身のnullチェックなんですが、それすらもダメ?

2018-03-14 21:31 Updated by: from
Comment

Debug 時のみ EnableReleaseOnFinalizer = true にするようアプリに記述しておけば、

DebugとReleaseとで、Finalizeのタイミングが違うし・・・とも思いましたが、Dispose漏れのログ出しという見地では大差ないか。

まあ実際 COMのRelease もしますけど、例の issue にあるとおり、何が起きるかわからない状態ですからねー。

EnableReleaseOnFinalizer は、ネイティブの解放の保証というよりも、Dispose漏れの検知にのみ使えるフラグだと思います。

ファイナライザ(~CTexture)からマネージド(this.texture)を参照するのはNGの方向で……(汗

本質的にはマネージドクラス変数(=参照ポインタ)自身のnullチェックなんですが、それすらもダメ?

this.texture がどう最適化されてどんな MSIL に変換されるかによりますので、やめておくのが無難なのでは……。

2018-03-14 22:43 Updated by: yyagi
Comment

ファイナライザ(~CTexture)からマネージド(this.texture)を参照するのはNGの方向で……(汗

本質的にはマネージドクラス変数(=参照ポインタ)自身のnullチェックなんですが、それすらもダメ?

this.texture がどう最適化されてどんな MSIL に変換されるかによりますので、やめておくのが無難なのでは……。

それならば、「this.textureをdisposeしたかどうかのbool変数」をつくって、それで判定しましょう。 ここまでやるのはどうかという気がしないでもないですが、SharpDX側のログだけだと、どのテクスチャの解放漏れ化が分かりにくいので。

2018-03-17 01:21 Updated by: yyagi
Comment

実際に実装して試してみました。

少なくともDEBUGビルドでは、トレースログのリスナーが開放された後でファイナライザが走ってますね。つまりDispose漏れがDTXManiaLog.txtには記録されない・・・。VisualStudioのデバッグ出力を見るか、DbgViewを使うかする必要があります。

なお DTXMania Rel112 で実際に動作確認してみたところ、いくつか開放漏れが見つかりました。といっても既知(#34196)の問題そのままなので、おいおい直しておきます。(影響度が小さい問題なので放置していました)

2018-09-01 01:19 Updated by: yyagi
  • Status Update from Open to Closed
Comment

CTexture() クラスそのものは修正が完了し、Release 113で修正版を公開済みのため、本チケットはクローズします。

メモリリークしているところは、CTexture()の呼び出し元で対策ください。

2020-01-08 02:36 Updated by: yyagi
  • Status Update from Closed to Open
Comment

更に、CTextureでのテクスチャ生成時に、分かりやすいラベルを付与できるようにしました。 メモリリークを検出した際に、そのラベルがデバッグログに出力されます。

Rev. 2033adeeee1e46267cf36b50919c9ecee8804076 で対応済み。

2020-01-20 00:49 Updated by: yyagi
  • Status Update from Open to Closed
Comment

Release117に取り込み済み。

Attachment File List

No attachments

Edit

You are not logged in. I you are not logged in, your comment will be treated as an anonymous post. » Login