Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Winでのマルチスレッドバグの可能性を検証する #232

Closed
koron opened this issue Aug 5, 2012 · 10 comments
Closed

Winでのマルチスレッドバグの可能性を検証する #232

koron opened this issue Aug 5, 2012 · 10 comments

Comments

@koron
Copy link
Member

koron commented Aug 5, 2012

os_win32.c:3564 の mch_system_piped() 内の、3665行目 CreateThread() 呼び出しをみて、ふと気がついた問題。system()をPIPEで実装し、そこへのバッファの書き出しを別スレッドでやっている、ように見える。

問題点の可能性

  • 書き出し中にバッファが書き換わる可能性はないか?
  • CreateThread() ではなく _beginthreadex() を使わなくて大丈夫か? (ランタイムの適切な初期化)

以上を検証したほうが良い。

現時点で問題が生じていなければさほど気にしなくても良いのですが、可能なら将来の問題の芽は摘みとっておきたいです。

@ynkdir
Copy link
Member

ynkdir commented Aug 5, 2012

書き出し中にバッファが書き換わる可能性

フィルタ実行中に外部から :call remote_expr('GVIM', 'setline("$", "x")') で書き換えできます。

@koron
Copy link
Member Author

koron commented Aug 5, 2012

まじか…どうすんだコレ…行数を変える系の操作がヤバ気。

@mattn
Copy link
Member

mattn commented Aug 6, 2012

すぐその下にループがあってPeekMessage(PM_REMOVE)して捨ててるから大丈夫そうな気がするんだが。

@ynkdir
Copy link
Member

ynkdir commented Aug 6, 2012

そのメッセージループでclient/server通信のメッセージを受信・処理してるので
バッファを読み込んでパイプへ書き込む処理(別スレッド)とremote_expr()で渡された式の実行処理(メインスレッド)が平行して走ります。
ただまあぶっちゃけ誰も困らないと思いますがいまのところ。

@k-takata
Copy link
Member

k-takata commented Aug 7, 2012

  • CreateThread() ではなく _beginthreadex() を使わなくて大丈夫か? (ランタイムの適切な初期化)

スレッドの中で初期化・終了処理が必要なCRT関数を使っていなければ大丈夫でしょうし、CRTにダイナミックリンクしていれば、終了処理は行われるので大丈夫なはずですが、大丈夫か気にするくらいなら _beginthreadex() にしてしまった方がよいのではという気もしますね。
ついでに _beginthread で grep すると if_sniff.c で、_beginthreadex() ではなく _beginthread() を使っているところがありました。戻り値のハンドルを使わないのであれば _beginthread() でも問題ないのですが、使っているようなので _beginthreadex() の方が良いでしょうね。

@koron
Copy link
Member Author

koron commented Sep 15, 2012

wontfix とします。

誰も困った様子がないので…困ったときになんとかしましょう。

@koron koron closed this as completed Sep 15, 2012
@k-takata
Copy link
Member

k-takata commented Aug 2, 2016

  • CreateThread() ではなく _beginthreadex() を使わなくて大丈夫か? (ランタイムの適切な初期化)

やっぱり気になったのでパッチを投げました。
https://groups.google.com/d/msg/vim_dev/Lx7-yflimq4/kNXrsGXjBgAJ
デフォルトのビルド設定だとCRTはスタティックリンクなので、TLSを使っているCRT関数を使っているとメモリリークしてしまいます。

@k-takata
Copy link
Member

k-takata commented Aug 2, 2016

7.4.2145
https://groups.google.com/d/msg/vim_dev/Dv8VQY1qELY/4sPXDfT6BgAJ
mingwでwarningが出るらしいので、あとで追加パッチを送付予定。

@k-takata
Copy link
Member

k-takata commented Aug 4, 2016

@k-takata
Copy link
Member

k-takata commented Aug 4, 2016

  • 書き出し中にバッファが書き換わる可能性はないか?

こちらは、困った人がいたらそのときに。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants