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

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
833e8b40131d9658beb8470e1b0cacf8?d=identicon&s=25 unknown (Guest)
on 2015-01-05 15:38
(Received via mailing list)
Issue #10697 has been reported by Takashi Sawanaka.

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

* 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が発生します。

```ruby
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/ma...
のように作成して 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 が起きるというように見えました。

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

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` のテストは通るのですが、以下のスクリプトを
実行するとメモリ使用量が増加し続けてしまいます。

```ruby
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`の時だけ特別扱いする
方法もあるかもしれません。


```C
--- 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)
This topic is locked and can not be replied to.