チケット #46136

Failing "15 > pos->ref_count" assert

登録: 2022-11-28 22:22 最終更新: 2022-12-17 10:01

報告者:
担当者:
チケットの種類:
状況:
完了
コンポーネント:
マイルストーン:
優先度:
5 - 中
重要度:
5 - 中
解決法:
修正済み
ファイル:
1

詳細

I've got a pf_fuel_pos_ref() assert against too big ref count failing. This is with an AI caravan traveling with a UTYF_COAST boat, i.e., combination of amphibious (land unit + sea unit) path finding and fuel path finding.

I'm not convinced that the assert is right. It seems to consider only the fact that one can enter the tile from 8 different directions (neighboring tiles) max. But won't also the refuel points count here? One can came from the same direction but with different refuel history. Of course the assert is right in that the 4 bit field can't hold bigger values than 15 -> maybe we need to make that wider.

OTOH amphibious PF makes some move cost multiplication magic that fuel path finding may not be completely compatible with.

チケットの履歴 (10 件中 3 件表示)

2022-11-28 22:22 更新者: cazfi
  • 新しいチケット "Failing "15 > pos->ref_count" assert" が作成されました
2022-12-01 12:04 更新者: cazfi
コメント

One bug there is that pf_fuel_pos cost overflows. It should be 'signed int' like the node cost that gets assigned to it. That's something we missed in #46039. While the field has always been too narrow, it was a dormant bug until #46039 fix made it to trigger.

As I encountered this in a ruleset with not-so-high granularity, it indicates that wider fields ( #46039 & co ) would actually be needed even for such rulesets (normal granularity, but with UTYF_COAST units) -> need to be backported to S3_0.

Changing that field to 'unsigned int' makes the autogame to go through without that failed assert. Yet, I don't think that's the actual bug of interest in this ticket. Seems more likely that fixing that part causes the autogame to proceed differently, so that it never triggers the assert case.

2022-12-01 13:53 更新者: cazfi
コメント

I *think* that there must be something wrong in the way pf_fuel_pos_replace() does not unref pos->prev when the ref_count of current pos > 1. Current pos gets replaced regardless of that ref_count, so the replaced pos is no longer there to ref prev (so why it's still included in the ref_count?)

Unfortunately lack of that unref might be integral part of the whole idea of pf_fuel_pos_replace(), and the bug not solvable by just adding unreffing. Need to investigate more.

2022-12-01 14:09 更新者: cazfi
コメント

Reply To cazfi

I *think* that there must be something wrong in the way pf_fuel_pos_replace() does not unref pos->prev when the ref_count of current pos > 1. Current pos gets replaced regardless of that ref_count, so the replaced pos is no longer there to ref prev (so why it's still included in the ref_count?) Unfortunately lack of that unref might be integral part of the whole idea of pf_fuel_pos_replace(), and the bug not solvable by just adding unreffing. Need to investigate more.

Finally got it. The current position is *not* completely lost in that case. A new one is just created in addition to it, to replace what node points to.

I think the summary is that 1) too narrow 'pos->cost' is the actual bug after all. 2) pf_fuel_pos_replace() comments (esp. function header) should be updated

2022-12-01 14:16 更新者: cazfi
コメント

Reply To cazfi

2) pf_fuel_pos_replace() comments (esp. function header) should be updated

-> #46149

2022-12-02 12:55 更新者: cazfi
  • 担当者(未割り当て) から cazfi に更新されました
  • 解決法なし から 受領 に更新されました
  • マイルストーン(未割り当て) から 3.1.0 (完了済み) に更新されました
コメント

Reply To cazfi

indicates that wider fields ( #46039 & co ) would actually be needed even for such rulesets (normal granularity, but with UTYF_COAST units) -> need to be backported to S3_0.

Targeting this to S3_1 at first.

2022-12-05 06:15 更新者: cazfi
コメント

maste/S3_1 patches pushed. Keeping ticket open for later backporting to S3_0.

2022-12-05 13:32 更新者: cazfi
  • 解決法なし から 受領 に更新されました
コメント

Current patch applies to S3_0 too.

2022-12-17 10:01 更新者: cazfi
  • 状況オープン から 完了 に更新されました
  • 解決法受領 から 修正済み に更新されました

添付ファイルリスト

編集

このチケットにコメントを追加するには、ログインが必要です » ログインする