[ruby-trunk - Bug #10697] WIN32OLE: WIN32OLE_RECORD を使用したスクリプト終了時にruby.exe がクラッシュすることがある

addis_a · January 8, 2015, 4:58pm

Issue #10697 has been updated by Masaki S…

Assignee changed from cruby-windows to Masaki S.

ありがとうございます。
現象自体は確認しており、メモリーの二重開放まではわかっていたのですが、
COMサーバ側で開放しているとは思いませんでした。
ちょっと時間が取れてなくてパッチの方は詳しく見ていないのですが、別のアプローチの
VT_BYREF|VT_RECORDで渡したら、こちらでも現象は起こらなくなったみたいです。
わざわざ、VT_VARIANT | VT_BYREF で渡しているのは参照渡しにしたいからなので、
参照渡しをするのなら、VT_RECORD|VT_BYREFでいいような気がしてきました。
IRecordInfo::RecordCreateで確保することは最初に実装するときに試してみたんですが
なんかうまくいかなくて、ALLOC_N()の確保に切り替えた記憶があります。
(記憶があやふやで正確なところが思いだせない。)
時間が取れるようになったら、もう少し調べてみます。

Bug #10697: WIN32OLE: WIN32OLE_RECORD を使用したスクリプト終了時にruby.exe
がクラッシュすることがある

Author: Takashi Sawanaka
Status: Open
Priority: Normal
Assignee: Masaki S.
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 に再代入するという方法もあるかもしれません)
1. で解放漏れが発生しないようするのと、COMサーバーが戻り値としてVT_RECORDタイプのVARIANTデータを返してきたとき、
  このVARIANT::pvRecord は WIN32OLE側で解放する責任がありそうなため、
  VariantClear() する箇所の大部分を VT_RECORD タイプのVARIANTデータならば
  IRecordInfo::RecordDestroy() で解放する新設関数 ole_variant_clear()
  に置き換えています。

なお、このパッチでは、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)