Issue #10697 has been reported by Takashi Sawanaka.
Bug #10697: WIN32OLE: WIN32OLE_RECORD を使用したスクリプト終了時にruby.exe
がクラッシュすることがある
- Author: Takashi Sawanaka
- Status: Open
- Priority: Normal
- Assignee: cruby-windows
- Category: platform/windows
- Target version: current: 2.2.0
- ruby -v: ruby 2.3.0dev (2015-01-03 trunk 49122) [i386-mswin32_120]
- Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN
以下のスクリプトまたは、Ruby のソース内のテスト test/win32ole/test_win32ole_record.rb
を実行すると数回に一度程度の確率で ruby プロセスの終了時にSEGVが発生します。
require 'win32ole'
obj = WIN32OLE.new('RbComTest.ComSrvTest')
book = WIN32OLE_RECORD.new('Book', obj)
obj.getBookByRefBook(book)
- ※1 上記スクリプトで使用している
RbComTest.ComSrvTestは、
test/win32ole/test_win32ole_record.rbファイル内に記述されている VB.NET COM server
を ビルドして作成しています。
私は、https://github.com/sdottaka/mruby-win32ole/tree/master/test/RbComTest
のように作成して VS2013 でビルドしました。 - ※2 実行環境は Windows 8.1 64bit版。 Visual Studio 2013 で ruby を32bitビルドしています。
異常発生時、VisualStudioで見たコールスタックは以下の通りです。
ntdll.dll!7764ebd8() Unknown
[Frames below may be incorrect and/or missing, no symbols loaded for
ntdll.dll]
ntdll.dll!775aa68c() Unknown
oleaut32.dll!7706df93() Unknown
msvcr120.dll!585aecfa() Unknown
> win32ole.so!olerecord_free(void * ptr) Line 220 C
msvcr120-ruby230.dll!finalize_list(rb_objspace * objspace, unsigned
long zombie) Line 2476 C
msvcr120-ruby230.dll!rb_objspace_call_finalizer(rb_objspace *
objspace) Line 2617 C
msvcr120-ruby230.dll!rb_gc_call_finalizer_at_exit() Line 2549 C
msvcr120-ruby230.dll!ruby_cleanup(volatile int ex) Line 233 C
msvcr120-ruby230.dll!ruby_run_node(void * n) Line 310 C
ruby.exe!main(int argc, char * * argv) Line 36 C
ruby.exe!__tmainCRTStartup() Line 626 C
kernel32.dll!7599919f() Unknown
ntdll.dll!775c0bbb() Unknown
ntdll.dll!775c0b91() Unknown
調査してみたところ、二重free が起きているように思われました。
上記スクリプトの4行目のobj.getBookByRefBook(book)が呼ばれると
win32ole.c の ole_invoke() 関数内の IDispatch::Invoke() に引数として渡している
VT_RECORD|VT_BYREFのVARIANT型引数(book引数に相当)のメンバ pvRecord が示すメモリが
COMサーバー側で解放＆再確保され、アドレスが書き換わってしまっています。(アドレスが変わらないこともあります)
この pvRecord の値は、WIN32OLE_RECORD 内で確保して保持しているメモリアドレス（struct olerecorddata::pdata)ですが、
上記 Invoke の呼び出しで解放と再確保されてアドレスが変わっているのに気付かず、 WIN32OLE_RECORD オブジェクト
の解放時に古いメモリアドレスでfreeしようとして2重free が起きるというように見えました。
余計なものだとは思いますが、ご参考までに修正しようとして挫折した途中のパッチを添付しています。
このパッチでは、
- 上記のように
WIN32OLE_RECORDが確保するstruct olerecorddata::pdata
のメモリがCOMサーバー側で解放されることがあるため、
メモリアロケータをCOMサーバー側のデアロケータと同じ種類のものにしないとまずそうです。
このため、ole_rec2variant()関数内で行っているメモリ確保をALLOC_N()ではなく、IRecordInfo::RecordCreate()に変更しています。 -
struct olerecorddata::pdataのメモリはWIN32OLE_RECORDオブジェクトをVARIANT型に変換するときにのみ必要なのと、
上記のように書き換えられてしまうためWIN32OLE_RECORD内で保持する必要がないと考え、
struct olerecorddata::pdataは削除し、変換後のVARIANT型データをクリアするときに
IRecordInfo::RecordDestroy()で解放するようにしています。
(再確保後のメモリアドレスをpdata に再代入するという方法もあるかもしれません) -
- で解放漏れが発生しないようするのと、COMサーバーが戻り値としてVT_RECORDタイプのVARIANTデータを返してきたとき、
このVARIANT::pvRecordは WIN32OLE側で解放する責任がありそうなため、
VariantClear()する箇所の大部分を VT_RECORD タイプのVARIANTデータ ならば
IRecordInfo::RecordDestroy()で解放する新設関数ole_variant_clear()
に置き換えています。
- で解放漏れが発生しないようするのと、COMサーバーが戻り値としてVT_RECORDタイプのVARIANTデータを返してきたとき、
なお、このパッチでは、test/win32ole/test_win32ole_record.rb のテストは通るのですが、以下のスクリプトを
実行するとメモリ使用量が増加し続けてしまいます。
require 'win32ole' unless Module.const_defined?(:WIN32OLE)
def test1
obj = WIN32OLE.new('RbComTest.ComSrvTest')
rec = WIN32OLE_RECORD.new('Book', obj)
rec.title = "Ruby Book"
rec.cost = 60
book = obj.getVer2BookByValBook(rec)
end
def test2
obj = WIN32OLE.new('RbComTest.ComSrvTest')
book = WIN32OLE_RECORD.new('Book', obj)
obj.getBookByRefBook(book)
end
10000000.times do
test1
test2
GC.start
end
また、良いかどうかわからないのですが、別のアプローチとして、IDispatch::Invoke に渡すVARIANT型のデータを
VT_VARIANT|VT_BYREF ではなく、 VT_RECORD|VT_BYREF にすると pvRecord が
再確保されない雰囲気があるので、これに頼って以下のようにVT_RECORDの時だけ特別扱いする
方法もあるかもしれません。
--- win32ole-53d9cb7-left.c Mon Jan 05 23:13:20 2015
+++ win32ole.c Mon Jan 05 23:14:32 2015
@@ -2665,8 +2665,13 @@
ole_variant2variant(param, &op.dp.rgvarg[n]);
} else {
ole_val2variant(param, &realargs[n]);
- V_VT(&op.dp.rgvarg[n]) = VT_VARIANT | VT_BYREF;
- V_VARIANTREF(&op.dp.rgvarg[n]) = &realargs[n];
+ if (realargs[n].vt == VT_RECORD) {
+ op.dp.rgvarg[n] = realargs[n];
+ V_VT(&op.dp.rgvarg[n]) = VT_RECORD | VT_BYREF;
+ } else {
+ V_VT(&op.dp.rgvarg[n]) = VT_VARIANT | VT_BYREF;
+ V_VARIANTREF(&op.dp.rgvarg[n]) = &realargs[n];
+ }
}
}
}
—Files--------------------------------
patch.txt (9.79 KB)