[Message Prev][Message Next][Thread Prev][Thread Next][Message Index][Thread Index]

Re: [MD:7279] チケット #237



やまざきともうします。

# 以下のパッチの作成者です。

Date: Fri, 21 Apr 2006 21:18:35 +0900 (JST)
From: "M.Fujii" <boochang@xxxxxxxxxxxx>
Subject: Re: [MD:7279] チケット #237
> 
> 藤井です。
> 
> From: Hideyuki SHIRAI (白井秀行) <shirai@xxxxxxxxxxx>
> Subject: [MD:7279] チケット #237
> Date: Fri, 21 Apr 2006 20:03:22 +0900 (JST)
> > どなたか、チケット #237 のパッチ査読してくださいな。
> > 
> > http://www.meadowy.org/meadow/ticket/237
> > 
> > remove-framehdc-00.patch (4.6 kB) - anonymous によって04/09/06 21:45:06に追加.
> >     FRAME_HDCの削除
> > remove-warning-00.patch (2.1 kB) - anonymous によって04/09/06 21:45:56に追加.
> >     Warningの解消パッチ
> > exclusion-01.patch (8.2 kB) - anonymous によって04/16/06 16:40:25に追加.
> >     排他追加(4/16版)
> > 
> > http://www.meadowy.org/meadow/attachment/ticket/237/remove-framehdc-00.patch?format=raw
> > http://www.meadowy.org/meadow/attachment/ticket/237/remove-warning-00.patch?format=raw
> > http://www.meadowy.org/meadow/attachment/ticket/237/exclusion-01.patch?format=raw
> > 
> > これらを当ててから、一回も落ちなくなりました。

パッチの内容についてコメントします。

・remove-framehdc-00.patch
GET/REALEASE_FRAME_HDCで排他制御をするようになってから
2回ほどマクロの変更漏れによって落ちたことがあったので、
全てGET/RELEASE_FRAME_HDCに置き換え、FRAME_HDCマクロを
削除するパッチになります。

・remove-warning-00.patch
手元(cygwin環境)でコンパイルしている最中でていたWarningを
つぶすためのパッチになります。
キャストの追加と、CVS HEADで対処されているものを取り込んでいます。
# まだWarningは残っていますが。

・exclusion-01.patch
message threadで実行されるexpose_frameとmain threadとの排他で
漏れていた区間への追加/必要がない区間の削除をおこなったパッチと
なります。

expose_frameでは基本的にはcurrent_matrixを元に再描画を実施しているので、
current_matrixを操作する部分で漏れていた部分に、排他制御のための
GET/RELASE_FRAME_HDCを追加しています。

face関連/mouse highlight関連も同様に必要なので追加しています。

window.cでCLEANUP_FRAME_HDCを追加していますが、どうも、HDCを
保持している際に、Fsignalなどで、対応がとれなくなることがあり、
Mewのスクロールでmain_thread_hdc_nestlevelがoverflowしてしまう
問題があり、ad-hocな対処として盛り込んでいます。
多分本当は、unwind_to_catchにあるUNBLOCK_INPUT_TOみたいな処理を
用意する必要があるのだと思います。

また、藤井さんのご指摘の関数の引数追加ですが、exclusion-00.patchでは、
引数を追加していましたが、白井さんのコメントでVC6ではコンパイル
できなかったとのことで、再確認したところ、上位で排他がかかっていたため、
exclusion-01.patchでは、この修正を取り消しています。

> パッチでの排他制御の妥当性については分かりませんが、非常に気になる点が
> あるので意見させてください。
> 
> このパッチでは関数に引数を追加してまで frame を排他処理を行なうべきとこ
> ろまで渡す処理が追加されています。このようなことまでしないと排他できな
> いのなら、排他処理と DC 取得を分離し、frame を引数に取る必要なく排他で
> きるように変更する必要があるのではないかと思います。
> 
> このままではいたるところで #ifdef MEADOW しなければならず、保守性に悪影
> 響を及ぼしそうな気がします。

とりあえず、当方の環境で落ちないようにするための対処ですので、
考慮漏れの点があるかと思いますが、よろしくおねがいします。

ちなみに、mouse highlightまわりは、まだ対処が不十分です。

expose_frameで呼ばれるnote_mouse_highlight内でtext_propertyを
確認するため、グローバル変数のcurrent_bufferなどを切り替えています。

- message thread側でこの区間
- main thread側でcode_convert_regionなどのcurrent_bufferに
  対する操作を行う関数が動作している区間

が非同期で同時に走行している際に、main/message threadの切り替えが
連続して発生すると、バッファ内容が異なる状態になり、バッファ表示が
おかしくなったり、落ちたりします。

当方の環境では、navi2chで、多数のスレッドの差分情報を更新(I)している
最中に、マウスを頻繁に動かして、mouse highlightが変化させると、たまに、
insert-file-contentsから呼ばれるcode_convert_regionで落ちたり、
画面表示がおかしくなります。

対処は、mouse highlight処理を全てmain_threadで実施するか、
message_threadでこの区間を処理中はスレッド切り替えが発生しないようにするか
などと思うのですが、よくわからないです。
(NTEmacsやMeadow2でもcurrent_bufferの排他はないようにみえるので、
発生するような気もするのですが。)

-- 
yamazaki-k(at)mwe.biglobe.ne.jp