ma8ma commented 2 weeks ago

書き込みログを開いて前のレスに移動するショートカットキーを使うと一時的にハングアップする

バグの説明

書き込みログを開いて前のレスに移動するショートカットキーを使うと一時的にハングアップすると5chに報告がありました。

https://mao.5ch.net/test/read.cgi/linux/1640504277/800-809n

再現の方法

準備
1. 新しいキャッシュディレクトリでJDimを起動する
```
$ JDIM_CACHE=/path/to/new/cache /path/to/jdim
```
  - /path/to/new/cache は適当な場所
  - /path/to/jdim はインストール、またはビルドしたjdimの実行ファイル
2. メニューバーの設定(C)＞一般(G)＞書き込みログを保存する(A)をチェックして有効にする。
3. 適当なスレを開き、書き込みを何度か行って書き込みログを貯める。
4. または、キャッシュディレクトリにあるlog/postlogを直接編集して書き込みログを水増しする。
バグの再現
1. 準備で作成したキャッシュディレクトリでJDimを起動する
```
$ JDIM_CACHE=/path/to/new/cache /path/to/jdim
```
2. メニューバーのツール(T)＞書き込みログの表示(P)をクリックして書き込みログを表示する
3. 書き込みログにスクロールバーが表示されているのを確認する。
4. スクロールバーがないときは、書き込みを増やす、またはウインドウのサイズを縮小してスクロールバーを表示させる。
5. 書き込みログを下にスクロールする。
6. キーボードのpキーを押して「前のレスに移動」を実行する
7. JDimがハングアップ(フリーズ)する 13秒ほどでハングアップから開放されたと報告にありました。コンパイラー最適化をoffにしたビルドで試すとハングアップしたままで復帰を確認できませんでした。

やりたかったこと・期待する結果

書き込みログでpキーを押して「前のレスに移動」を実行してもフリーズしない。

スクリーンショット

なし

動作環境

[バージョン] JDim 0.12.0-beta20240617(git:67fc818407)
[ディストリ ] Ubuntu 24.04 LTS (x86_64)
[パッケージ] バイナリ/ソース( <配布元> )
[ DE／WM ] KDE
[　gtkmm 　] 3.24.9
[　glibmm 　] 2.66.7
[　TLS lib　] GnuTLS 3.8.3
[オプション ] '--with-alsa''--with-pangolayout'
[ そ の 他 ]

追加の情報

なし

mtasaka commented 2 weeks ago

(gdb) bt
#0  std::_Tuple_impl<0ul, ARTICLE::LayoutTree*, std::default_delete<ARTICLE::LayoutTree> >::_M_head (__t=...) at /usr/include/c++/14/tuple:296
#1  std::__get_helper<0ul, ARTICLE::LayoutTree*, std::default_delete<ARTICLE::LayoutTree> > (__t=...) at /usr/include/c++/14/tuple:2433
#2  std::get<0ul, ARTICLE::LayoutTree*, std::default_delete<ARTICLE::LayoutTree> > (__t=<optimized out>) at /usr/include/c++/14/tuple:2450
#3  std::__uniq_ptr_impl<ARTICLE::LayoutTree, std::default_delete<ARTICLE::LayoutTree> >::_M_ptr (this=<optimized out>) at /usr/include/c++/14/bits/unique_ptr.h:193
#4  std::unique_ptr<ARTICLE::LayoutTree, std::default_delete<ARTICLE::LayoutTree> >::get (this=<optimized out>) at /usr/include/c++/14/bits/unique_ptr.h:464
#5  std::unique_ptr<ARTICLE::LayoutTree, std::default_delete<ARTICLE::LayoutTree> >::operator-> (this=<optimized out>) at /usr/include/c++/14/bits/unique_ptr.h:457
#6  ARTICLE::DrawAreaBase::goto_pre_res (this=0x51c000090080) at ../src/article/drawareabase.cpp:3457
#7  ARTICLE::DrawAreaBase::set_scroll (this=0x51c000090080, control=<optimized out>) at ../src/article/drawareabase.cpp:3020
#8  0x0000000000af38f1 in ARTICLE::ArticleViewBase::operate_view (this=0x519000ba9580, control=117) at ../src/article/articleviewbase.cpp:1045
#9  0x0000000000cc89b9 in sigc::internal::signal_emit1<bool, _GdkEventScroll*, sigc::nil>::emit (impl=<optimized out>, _A_a1=<optimized out>)
    at /usr/include/sigc++-2.0/sigc++/signal.h:948
#10 0x0000000000ccb9a2 in sigc::signal1<bool, _GdkEventKey*, sigc::nil>::emit (this=<optimized out>, _A_a1=@0x7f215f421ce0: 0x50e00097d900)
    at /usr/include/sigc++-2.0/sigc++/signal.h:2961
#11 ARTICLE::DrawAreaBase::slot_key_press_event (this=0x51c000090080, event=<optimized out>) at ../src/article/drawareabase.cpp:5236
#12 0x00007f2164d18320 in (anonymous namespace)::Widget_signal_event_callback(_GtkWidget*, _GdkEvent*, void*) [clone .lto_priv.0] () at /lib64/libgtkmm-3.0.so.1
#13 0x00007f216408d614 in _gtk_marshal_BOOLEAN__BOXED
    (closure=0x507000520b50, return_value=0x7ffdcb944860, param_values=0x7ffdcb9448f0, marshal_data=<optimized out>, invocation_hint=<optimized out>, n_param_values=<optimized out>) at gtk/gtkmarshalers.c:84
#14 0x00007f21647cc64a in g_closure_invoke (closure=0x507000520b50, return_value=0x7ffdcb944860, n_param_values=2, param_values=0x7ffdcb9448f0, invocation_hint=0x7ffdcb944840)
    at ../gobject/gclosure.c:834
#15 0x00007f21647fc94d in signal_emit_unlocked_R.isra.0
    (node=node@entry=0x7ffdcb9449f0, detail=detail@entry=0, instance=instance@entry=0x5120002f6530, emission_return=emission_return@entry=0x7ffdcb944a70, instance_and_params=instance_and_params@entry=0x7ffdcb9448f0) at ../gobject/gsignal.c:3961
#16 0x00007f21647ec969 in signal_emit_valist_unlocked
    (instance=instance@entry=0x5120002f6530, signal_id=signal_id@entry=68, detail=detail@entry=0, var_args=var_args@entry=0x7ffdcb944b60) at ../gobject/gsignal.c:3533
#17 0x00007f21647ed361 in g_signal_emit_valist (instance=0x5120002f6530, signal_id=68, detail=0, var_args=var_args@entry=0x7ffdcb944b60) at ../gobject/gsignal.c:3263
#18 0x00007f21647ed423 in g_signal_emit (instance=instance@entry=0x5120002f6530, signal_id=<optimized out>, detail=detail@entry=0) at ../gobject/gsignal.c:3583
#19 0x00007f216435d2f4 in gtk_widget_event_internal.part.0.lto_priv.0 (widget=0x5120002f6530, event=0x50e00097d900) at ../gtk/gtkwidget.c:7812
#20 0x00007f216436c8bb in gtk_window_propagate_key_event (window=0x51700001fe60, event=0x50e00097d900) at ../gtk/gtkwindow.c:8259
#21 0x0000000001938f6d in SKELETON::JDWindow::on_key_press_event (this=0x51700001f880, event=0x50e00097d900) at ../src/skeleton/window.cpp:749
#22 0x00007f2164d25081 in Gtk::Widget_Class::key_press_event_callback(_GtkWidget*, _GdkEventKey*) () at /lib64/libgtkmm-3.0.so.1
#27 0x00007f21647ed423 in <emit signal '???' on instance ???> (instance=instance@entry=0x51700001fe60, signal_id=<optimized out>, detail=detail@entry=0)
    at ../gobject/gsignal.c:3583
    #23 0x00007f21640888d9 in _gtk_marshal_BOOLEAN__BOXEDv
    (closure=0x5060000095e0, return_value=0x7ffdcb944ee0, instance=<optimized out>, args=<optimized out>, marshal_data=<optimized out>, n_params=<optimized out>, param_types=0x50200000ea90) at gtk/gtkmarshalers.c:130
    #24 0x00007f21647ed254 in _g_closure_invoke_va
    (closure=0x5060000095e0, return_value=0x7ffdcb944ee0, instance=0x51700001fe60, args=0x7ffdcb944fd0, n_params=<optimized out>, param_types=0x50200000ea90)
    at ../gobject/gclosure.c:897
    #25 signal_emit_valist_unlocked (instance=instance@entry=0x51700001fe60, signal_id=signal_id@entry=68, detail=detail@entry=0, var_args=var_args@entry=0x7ffdcb944fd0)
    at ../gobject/gsignal.c:3424
    #26 0x00007f21647ed361 in g_signal_emit_valist (instance=0x51700001fe60, signal_id=68, detail=0, var_args=var_args@entry=0x7ffdcb944fd0) at ../gobject/gsignal.c:3263
#28 0x00007f216435d2f4 in gtk_widget_event_internal.part.0.lto_priv.0 (widget=0x51700001fe60, event=0x50e00097d900) at ../gtk/gtkwidget.c:7812
--Type <RET> for more, q to quit, c to continue without paging--
#29 0x00007f21641f2f97 in propagate_event (widget=0x51700001fe60, event=0x50e00097d900, captured=<optimized out>, topmost=0x0) at ../gtk/gtkmain.c:2681
#30 0x00007f21641f3c0a in gtk_main_do_event (event=<optimized out>) at ../gtk/gtkmain.c:1921
#31 gtk_main_do_event (event=<optimized out>) at ../gtk/gtkmain.c:1691
#32 0x00007f2163f34757 in _gdk_event_emit (event=0x50e00097d900) at ../gdk/gdkevents.c:73
#33 _gdk_event_emit (event=0x50e00097d900) at ../gdk/gdkevents.c:67
#34 0x00007f2163f8eabe in gdk_event_source_dispatch.lto_priv () at ../gdk/x11/gdkeventsource.c:354
#35 0x00007f2163be4e8c in g_main_dispatch (context=0x510000000640) at ../glib/gmain.c:3344
#36 g_main_context_dispatch_unlocked (context=0x510000000640) at ../glib/gmain.c:4152
#37 0x00007f2163c46c98 in g_main_context_iterate_unlocked.isra.0 (context=context@entry=0x510000000640, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>)
    at ../glib/gmain.c:4217
#38 0x00007f2163be6383 in g_main_context_iteration (context=context@entry=0x510000000640, may_block=may_block@entry=1) at ../glib/gmain.c:4282
#39 0x00007f2162b130fd in g_application_run (application=0x512000003440, argc=<optimized out>, argv=0x7ffdcb946598) at ../gio/gapplication.c:2712
#40 0x000000000059ed37 in App::run (this=0x7f215fa098e0, argc=1, argv=0x7ffdcb946598) at ../src/main.cpp:234
#41 main (argc=<optimized out>, argv=<optimized out>) at ../src/main.cpp:554

mtasaka commented 2 weeks ago

ここが無限loopになっている？

https://github.com/JDimproved/JDim/blob/67fc8184072d65d23fb720805c54f731f3e6cf47/src/article/drawareabase.cpp#L3457

mtasaka commented 2 weeks ago

#6  ARTICLE::DrawAreaBase::goto_pre_res (this=0x51c000090080) at ../src/article/drawareabase.cpp:3457
3457        do{ header = m_layout_tree->get_header_of_res_const( --num ); } while( num && ( ! header || header->rect->y >= pos_y ) );
(gdb) p num
$1 = -1987521278
(gdb) p m_seen_current
warning: RTTI symbol not found for class 'ARTICLE::DrawAreaMain [clone .lto_priv.0]'
$2 = 0

多分 m_seen_current == 0 の時の処理が良くない。関連するのは git blame するとこれ https://github.com/JDimproved/JDim/commit/3866ac88eaad35b52faffa0b7540b5dfcfc6d76d 変更前だと、上の場合loopに入らない

mtasaka commented 2 weeks ago

そもそも「書き込みログ」の場合、res番がなくて、m_seen_current とか max_number() とかが常に0（要は実質実装されてない）とかいう事はないですかね？外してるかも知れないけど...

pの代わりにnを押すと、常に一番下までscrollするように見えます。

ma8ma commented 2 weeks ago

@mtasaka バックトレースの調査ありがとうございます。通常のスレは最低でもレスが1つあるので（m_seen_currentが1以上）それを前提に修正されていますね。

@mtasaka さんの言うとおり、書き込みログの表示ではレス番号の計算は行わないようなのでARTICLE::DrawAreaBase::goto_pre_res()のローカル変数numは常にゼロになりますね。

ループの中にある--numで負の数になりループ条件のnumがtrueとして評価される
LayoutTree::get_header_of_res_const()に負の数や範囲外のレス番号を渡すとnullptrを返してheaderに代入されるがこれもループ条件でtrueになる
numをデクリメントし続けてアンダーフローしてさらにゼロになるまで続けるとようやくループから脱出できる

なのでwhile条件文のnumを修正して0 < numにすると無限ループは解決しそうですね。ショートカットキーを押しても何もしなくなりますが…うーん、挙動をどうするか悩ましい

書き込みログでスレビューのショートカットキーが使えるのはハプニングなので何もしないままにする
書き込みログでは逆方向の操作「次のレスに移動」が一番下にスクロールするので「前のレスに移動」は一番上にスクロールさせる
通常のスレの前/次のレスに移動と同じように挙動を実装する（レス番号の計算を実装しないといけない）

mtasaka commented 2 weeks ago

やはり取り敢えず考えられるのはその3案ですよね。

ただ最後の案は一番真面目だけど、時間がかかりそうです。少なくとも現在post betaなので、やるにしても直近のリリース後の次のphase送りでいいと思います。
最初の2案のどちらでもいい気がしますが、2番目の「一番上にスクロール」が現在の挙動からすると「釣り合いが取れている」気がします（最初の案なら、nをした時も何もしないようにする必要がありそうです）。ただ、どちらでもいい気がします。

ma8ma commented 2 weeks ago

https://github.com/JDimproved/JDim/blob/67fc8184072d65d23fb720805c54f731f3e6cf47/src/article/drawareabase.cpp#L3438-L3445

次のレスに移動の実装を見るとレス番号を計算しないときに一番下にスクロールするのは書かれた通りの結果で偶発的な動作ではないようですね。 ARTICLE::DrawAreaBase::goto_pre_res()の冒頭にも同じ条件で一番上に移動する処理を追加すると挙動が対になるのでとりあえずは良さそうです。

ma8ma commented 1 week ago

https://mao.5ch.net/test/read.cgi/linux/1640504277/827

[ Linux ] JDim Part9【ﾜｯﾁｮｲ有】

827 名前：login:Penguin (ﾜｯﾁｮｲ 0fb8-Wcph) [sage]： 2024/06/23(日) 22:32:22.37 ID:ybzBFzBG0 (2/2) たびたびすみません

0.12.0-beta20240617 ではスレ画面で一番下の書き込みに移動後「p」を押すと1つづつ前のレスに移動していたのですが 0.12.0-beta20240622 では「p」でスレのトップに移動するようになりました

0.12.0-beta20240617 のように1つづつ前のレスに移動出来るようになりますでしょうか

1405 の修正で通常のスレのショートカットキーまで動作が変わったため再度オープンして修正します。

JDimproved / JDim

書き込みログを開いて前のレスに移動するショートカットキーを使うと一時的にハングアップする #1404

1405 の修正で通常のスレのショートカットキーまで動作が変わったため再度オープンして修正します。