Project

General

Profile

Bug #10697

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

Added by sdottaka (Takashi Sawanaka) over 5 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Normal
Target version:
ruby -v:
ruby 2.3.0dev (2015-01-03 trunk 49122) [i386-mswin32_120]
[ruby-dev:48801]

Description

以下のスクリプトまたは、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_BYREFVARIANT型引数(book引数に相当)のメンバ pvRecord が示すメモリが
COMサーバー側で解放&再確保され、アドレスが書き換わってしまっています。(アドレスが変わらないこともあります)

この pvRecord の値は、WIN32OLE_RECORD 内で確保して保持しているメモリアドレス(struct olerecorddata::pdata)ですが、
上記 Invoke の呼び出しで解放と再確保されてアドレスが変わっているのに気付かず、 WIN32OLE_RECORD オブジェクト
の解放時に古いメモリアドレスでfreeしようとして2重free が起きるというように見えました。

余計なものだとは思いますが、ご参考までに修正しようとして挫折した途中のパッチを添付しています。
このパッチでは、

  1. 上記のように WIN32OLE_RECORDが確保する struct olerecorddata::pdata のメモリがCOMサーバー側で解放されることがあるため、 メモリアロケータをCOMサーバー側のデアロケータと同じ種類のものにしないとまずそうです。 このため、ole_rec2variant()関数内で行っているメモリ確保をALLOC_N()ではなく、IRecordInfo::RecordCreate()に変更しています。
  2. struct olerecorddata::pdataのメモリはWIN32OLE_RECORDオブジェクトをVARIANT型に変換するときにのみ必要なのと、 上記のように書き換えられてしまうためWIN32OLE_RECORD内で保持する必要がないと考え、 struct olerecorddata::pdata は削除し、変換後のVARIANT型データをクリアするときに IRecordInfo::RecordDestroy()で解放するようにしています。 (再確保後のメモリアドレスをpdata に再代入するという方法もあるかもしれません)
  3. 2. で解放漏れが発生しないようするのと、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) patch.txt sdottaka (Takashi Sawanaka), 01/05/2015 02:33 PM

Updated by suke (Masaki Suketa) over 5 years ago

  • Assignee changed from cruby-windows to suke (Masaki Suketa)

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

Updated by suke (Masaki Suketa) over 5 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

Applied in changeset r49315.


  • ext/win32ole/win32ole.c (ole_invoke): avoid SEGV when VT_RECORD variable is passed by reference. [ruby-dev:48803] [Bug #10697]
#3

Updated by usa (Usaku NAKAMURA) over 5 years ago

  • Backport changed from 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN to 2.0.0: DONTNEED, 2.1: DONTNEED, 2.2: REQUIRED
#4

Updated by naruse (Yui NARUSE) over 5 years ago

  • Backport changed from 2.0.0: DONTNEED, 2.1: DONTNEED, 2.2: REQUIRED to 2.0.0: DONTNEED, 2.1: DONTNEED, 2.2: DONE

ruby_2_2 r49352 merged revision(s) 49315.

Also available in: Atom PDF