[ruby-trunk - Bug #6911][Open] Sync m#sync unlock で ThreadError が発生する場合がある

aris · August 27, 2012, 2:01pm

Issue #6911 has been reported by mrkn (Kenta M.).

Bug #6911: Sync_m#sync_unlock で ThreadError が発生する場合がある

Author: mrkn (Kenta M.)
Status: Open
Priority: Normal
Assignee:
Category: lib
Target version: 2.0.0
ruby -v: ruby 2.0.0dev (2012-08-23 trunk 36795) [x86_64-darwin11.4.0]

Sync_m#sync_unlock で ThreadError が発生する場合があります。
原因は Sync_m#sync_lock が同一スレッドを複数回 sync_waiting 配列に push してしまうからです。

同一スレッドが多重に sync_waiting 配列に入っていると、sync_unlock の中で何度もそのスレッドに対して
Thread#run が呼ばれます。
すると、1回目の Thread#run でそのスレッドが終了した場合、2回目の Thread#run で ThreadError が発生します。

sync_waiting に同一スレッドが複数回 push された状態を再現するコードを gist に置きました。

gist.github.com

https://gist.github.com/mrkn/3434046

00result

$ make -C .build test-all TESTS='test_sync.rb'
        CC = clang
        LD = ld
        LDSHARED = clang -dynamiclib
        CFLAGS = -O0 -march=core2 -mtune=core2 -ggdb -gdwarf-2 -Wall -Wextra -Wno-unused-parameter -Wno-parentheses -Wno-long-long -Wno-missing-field-initializers -Wunused-variable -Werror=pointer-arith -Werror=write-strings -Werror=declaration-after-statement -Werror=shorten-64-to-32 -Werror=implicit-function-declaration -fno-common -pipe -arch x86_64
        XCFLAGS = -include ruby/config.h -include ruby/missing.h -D_FORTIFY_SOURCE=2 -fstack-protector -fno-strict-overflow -fvisibility=hidden -DRUBY_EXPORT
        CPPFLAGS = -D_XOPEN_SOURCE -D_DARWIN_C_SOURCE -D_DARWIN_UNLIMITED_SELECT -D_REENTRANT   -I. -I.ext/include/x86_64-darwin11.4.0 -I../include -I..
        DLDFLAGS = -Wl,-undefined,dynamic_lookup -Wl,-multiply_defined,suppress -install_name /Users/kenta-murata/work/ruby.git/.build/../.prefix/lib/libruby.2.0.0.dylib -current_version 2.0.0 -compatibility_version 2.0.0 -Wl,-unexported_symbol,_Init_* -Wl,-unexported_symbol,*_threadptr_*  -fstack-protector -Wl,-u,_objc_msgSend  -fstack-protector -Wl,-u,_objc_msgSend  -arch x86_64
        SOLIBS = 
./miniruby -I../lib -I. -I.ext/common  ../tool/runruby.rb --extout=.ext  -- --disable-gems "../test/runner.rb" --ruby="./miniruby -I../lib -I. -I.ext/common  ../tool/runruby.rb --extout=.ext  -- --disable-gems"  test_sync.rb

This file has been truncated. show original

sync.patch

diff --git a/lib/sync.rb b/lib/sync.rb
index bae05a4..c6a7321 100644
--- a/lib/sync.rb
+++ b/lib/sync.rb
@@ -147,7 +147,7 @@ module Sync_m
             sync_upgrade_waiting.push [Thread.current, sync_sh_locker[Thread.current]]
             sync_sh_locker.delete(Thread.current)
           else
-            sync_waiting.push Thread.current
+            sync_waiting.push Thread.current unless sync_waiting.include? Thread.current

This file has been truncated. show original

test_sync.rb

require 'test/unit'
require 'sync'
require 'timeout'

class SyncTest < Test::Unit::TestCase
  class Tester
    include Sync_m
  end

  def test_synchronize

This file has been truncated. show original

この gist に添付されてる test を実行すると、以下のような結果が得られると思います。

[1/1] SyncTest#test_synchronize = 0.51 s

Failure:
test_synchronize(SyncTest)
[/Users/kenta-murata/work/ruby.git/test/test_sync.rb:57]:
<[#<Thread:0x007f904183aa08 sleep>,
#<Thread:0x007f904183ab98 sleep>,
#<Thread:0x007f904183af58 sleep>]> expected but was
<[#<Thread:0x007f904183aa08 sleep>,
#<Thread:0x007f904183ab98 sleep>,
#<Thread:0x007f904183af58 sleep>,
#<Thread:0x007f904183aa08 sleep>]>.

修正用のパッチも上記 gist に添付してあります。修正方法がこの通りで良ければコミットしたいので、どなたかレビューをお願いします。

mrkn_SKenta_M.S · September 9, 2012, 1:28pm

Issue #6911 has been updated by kosaki (Motohiro KOSAKI).

Status changed from Open to Closed

調査した結果 5355と同じだと分かったのであっちのパッチいれます。ごめんね

Bug #6911: Sync_m#sync_unlock で ThreadError が発生する場合がある

Author: mrkn (Kenta M.)
Status: Closed
Priority: Normal
Assignee:
Category: lib
Target version: 2.0.0
ruby -v: ruby 2.0.0dev (2012-08-23 trunk 36795) [x86_64-darwin11.4.0]

Sync_m#sync_unlock で ThreadError が発生する場合があります。
原因は Sync_m#sync_lock が同一スレッドを複数回 sync_waiting 配列に push してしまうからです。

同一スレッドが多重に sync_waiting 配列に入っていると、sync_unlock の中で何度もそのスレッドに対して
Thread#run が呼ばれます。
すると、1回目の Thread#run でそのスレッドが終了した場合、2回目の Thread#run で ThreadError が発生します。

sync_waiting に同一スレッドが複数回 push された状態を再現するコードを gist に置きました。

gist.github.com

https://gist.github.com/mrkn/3434046

00result

$ make -C .build test-all TESTS='test_sync.rb'
        CC = clang
        LD = ld
        LDSHARED = clang -dynamiclib
        CFLAGS = -O0 -march=core2 -mtune=core2 -ggdb -gdwarf-2 -Wall -Wextra -Wno-unused-parameter -Wno-parentheses -Wno-long-long -Wno-missing-field-initializers -Wunused-variable -Werror=pointer-arith -Werror=write-strings -Werror=declaration-after-statement -Werror=shorten-64-to-32 -Werror=implicit-function-declaration -fno-common -pipe -arch x86_64
        XCFLAGS = -include ruby/config.h -include ruby/missing.h -D_FORTIFY_SOURCE=2 -fstack-protector -fno-strict-overflow -fvisibility=hidden -DRUBY_EXPORT
        CPPFLAGS = -D_XOPEN_SOURCE -D_DARWIN_C_SOURCE -D_DARWIN_UNLIMITED_SELECT -D_REENTRANT   -I. -I.ext/include/x86_64-darwin11.4.0 -I../include -I..
        DLDFLAGS = -Wl,-undefined,dynamic_lookup -Wl,-multiply_defined,suppress -install_name /Users/kenta-murata/work/ruby.git/.build/../.prefix/lib/libruby.2.0.0.dylib -current_version 2.0.0 -compatibility_version 2.0.0 -Wl,-unexported_symbol,_Init_* -Wl,-unexported_symbol,*_threadptr_*  -fstack-protector -Wl,-u,_objc_msgSend  -fstack-protector -Wl,-u,_objc_msgSend  -arch x86_64
        SOLIBS = 
./miniruby -I../lib -I. -I.ext/common  ../tool/runruby.rb --extout=.ext  -- --disable-gems "../test/runner.rb" --ruby="./miniruby -I../lib -I. -I.ext/common  ../tool/runruby.rb --extout=.ext  -- --disable-gems"  test_sync.rb

This file has been truncated. show original

sync.patch

diff --git a/lib/sync.rb b/lib/sync.rb
index bae05a4..c6a7321 100644
--- a/lib/sync.rb
+++ b/lib/sync.rb
@@ -147,7 +147,7 @@ module Sync_m
             sync_upgrade_waiting.push [Thread.current, sync_sh_locker[Thread.current]]
             sync_sh_locker.delete(Thread.current)
           else
-            sync_waiting.push Thread.current
+            sync_waiting.push Thread.current unless sync_waiting.include? Thread.current

This file has been truncated. show original

test_sync.rb

require 'test/unit'
require 'sync'
require 'timeout'

class SyncTest < Test::Unit::TestCase
  class Tester
    include Sync_m
  end

  def test_synchronize

This file has been truncated. show original

この gist に添付されてる test を実行すると、以下のような結果が得られると思います。

[1/1] SyncTest#test_synchronize = 0.51 s

Failure:
test_synchronize(SyncTest)
[/Users/kenta-murata/work/ruby.git/test/test_sync.rb:57]:
<[#<Thread:0x007f904183aa08 sleep>,
#<Thread:0x007f904183ab98 sleep>,
#<Thread:0x007f904183af58 sleep>]> expected but was
<[#<Thread:0x007f904183aa08 sleep>,
#<Thread:0x007f904183ab98 sleep>,
#<Thread:0x007f904183af58 sleep>,
#<Thread:0x007f904183aa08 sleep>]>.

修正用のパッチも上記 gist に添付してあります。修正方法がこの通りで良ければコミットしたいので、どなたかレビューをお願いします。

mrkn_SKenta_M.S · September 9, 2012, 1:34pm

Issue #6911 has been updated by kosaki (Motohiro KOSAKI).

余談なんですけど、なんで Mutex.sleep を thread.run で起こせる仕様なんでしたっけ？ Thread.stop,
Thread.sleep 以外は起こせないほうが普通の感覚かと思うのですが。
とはいえ、スレッドライブラリはTimeout
moduleとかでexeptionが突然飛んできても大丈夫なように書いてあるべきなので、この制限をいれてもSync_mを直さないといけないという結論は変わらないのですが

Bug #6911: Sync_m#sync_unlock で ThreadError が発生する場合がある

Author: mrkn (Kenta M.)
Status: Closed
Priority: Normal
Assignee:
Category: lib
Target version: 2.0.0
ruby -v: ruby 2.0.0dev (2012-08-23 trunk 36795) [x86_64-darwin11.4.0]

Sync_m#sync_unlock で ThreadError が発生する場合があります。
原因は Sync_m#sync_lock が同一スレッドを複数回 sync_waiting 配列に push してしまうからです。

同一スレッドが多重に sync_waiting 配列に入っていると、sync_unlock の中で何度もそのスレッドに対して
Thread#run が呼ばれます。
すると、1回目の Thread#run でそのスレッドが終了した場合、2回目の Thread#run で ThreadError が発生します。

sync_waiting に同一スレッドが複数回 push された状態を再現するコードを gist に置きました。

gist.github.com

https://gist.github.com/mrkn/3434046

00result

$ make -C .build test-all TESTS='test_sync.rb'
        CC = clang
        LD = ld
        LDSHARED = clang -dynamiclib
        CFLAGS = -O0 -march=core2 -mtune=core2 -ggdb -gdwarf-2 -Wall -Wextra -Wno-unused-parameter -Wno-parentheses -Wno-long-long -Wno-missing-field-initializers -Wunused-variable -Werror=pointer-arith -Werror=write-strings -Werror=declaration-after-statement -Werror=shorten-64-to-32 -Werror=implicit-function-declaration -fno-common -pipe -arch x86_64
        XCFLAGS = -include ruby/config.h -include ruby/missing.h -D_FORTIFY_SOURCE=2 -fstack-protector -fno-strict-overflow -fvisibility=hidden -DRUBY_EXPORT
        CPPFLAGS = -D_XOPEN_SOURCE -D_DARWIN_C_SOURCE -D_DARWIN_UNLIMITED_SELECT -D_REENTRANT   -I. -I.ext/include/x86_64-darwin11.4.0 -I../include -I..
        DLDFLAGS = -Wl,-undefined,dynamic_lookup -Wl,-multiply_defined,suppress -install_name /Users/kenta-murata/work/ruby.git/.build/../.prefix/lib/libruby.2.0.0.dylib -current_version 2.0.0 -compatibility_version 2.0.0 -Wl,-unexported_symbol,_Init_* -Wl,-unexported_symbol,*_threadptr_*  -fstack-protector -Wl,-u,_objc_msgSend  -fstack-protector -Wl,-u,_objc_msgSend  -arch x86_64
        SOLIBS = 
./miniruby -I../lib -I. -I.ext/common  ../tool/runruby.rb --extout=.ext  -- --disable-gems "../test/runner.rb" --ruby="./miniruby -I../lib -I. -I.ext/common  ../tool/runruby.rb --extout=.ext  -- --disable-gems"  test_sync.rb

This file has been truncated. show original

sync.patch

diff --git a/lib/sync.rb b/lib/sync.rb
index bae05a4..c6a7321 100644
--- a/lib/sync.rb
+++ b/lib/sync.rb
@@ -147,7 +147,7 @@ module Sync_m
             sync_upgrade_waiting.push [Thread.current, sync_sh_locker[Thread.current]]
             sync_sh_locker.delete(Thread.current)
           else
-            sync_waiting.push Thread.current
+            sync_waiting.push Thread.current unless sync_waiting.include? Thread.current

This file has been truncated. show original

test_sync.rb

require 'test/unit'
require 'sync'
require 'timeout'

class SyncTest < Test::Unit::TestCase
  class Tester
    include Sync_m
  end

  def test_synchronize

This file has been truncated. show original

この gist に添付されてる test を実行すると、以下のような結果が得られると思います。

[1/1] SyncTest#test_synchronize = 0.51 s

Failure:
test_synchronize(SyncTest)
[/Users/kenta-murata/work/ruby.git/test/test_sync.rb:57]:
<[#<Thread:0x007f904183aa08 sleep>,
#<Thread:0x007f904183ab98 sleep>,
#<Thread:0x007f904183af58 sleep>]> expected but was
<[#<Thread:0x007f904183aa08 sleep>,
#<Thread:0x007f904183ab98 sleep>,
#<Thread:0x007f904183af58 sleep>,
#<Thread:0x007f904183aa08 sleep>]>.

修正用のパッチも上記 gist に添付してあります。修正方法がこの通りで良ければコミットしたいので、どなたかレビューをお願いします。