You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "freemandealer (via GitHub)" <gi...@apache.org> on 2026/02/05 08:06:35 UTC
[PR] [fix](filecache) avoid SIGSEGV in background LRU update when clear cache [doris]
freemandealer opened a new pull request, #60533:
URL: https://github.com/apache/doris/pull/60533
The background LRU update thread (BlockFileCache::run_background_block_lru_update) uses cell which is a raw pointer owned by the cache’s internal _files map and may be unset during clearing cache.
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg: https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](filecache) avoid SIGSEGV in background LRU update when clear cache [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60533:
URL: https://github.com/apache/doris/pull/60533#issuecomment-3854284239
# BE Regression && UT Coverage Report
Increment line coverage `100.00% (14/14)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60533_86c0a22d28ad54ddd809f806891a3ac1e729508c_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60533_86c0a22d28ad54ddd809f806891a3ac1e729508c_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 71.56% (25850/36124) |
| Line Coverage | 54.20% (269976/498085) |
| Region Coverage | 51.68% (224742/434878) |
| Branch Coverage | 53.11% (96438/181570) |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](filecache) avoid SIGSEGV in background LRU update when clear cache [doris]
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on PR #60533:
URL: https://github.com/apache/doris/pull/60533#issuecomment-3877646992
run nonConcurrent
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](filecache) avoid SIGSEGV in background LRU update when clear cache [doris]
Posted by "Thearas (via GitHub)" <gi...@apache.org>.
Thearas commented on PR #60533:
URL: https://github.com/apache/doris/pull/60533#issuecomment-3851699065
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR).
Please clearly describe your PR:
1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](filecache) avoid SIGSEGV in background LRU update when clear cache [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60533:
URL: https://github.com/apache/doris/pull/60533#issuecomment-3853148394
# BE UT Coverage Report
Increment line coverage `100.00% (14/14)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/86c0a22d28ad54ddd809f806891a3ac1e729508c_86c0a22d28ad54ddd809f806891a3ac1e729508c/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/86c0a22d28ad54ddd809f806891a3ac1e729508c_86c0a22d28ad54ddd809f806891a3ac1e729508c/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 52.55% (19371/36861) |
| Line Coverage | 36.04% (179953/499273) |
| Region Coverage | 32.43% (139585/430460) |
| Branch Coverage | 33.41% (60425/180838) |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](filecache) avoid SIGSEGV in background LRU update when clear cache [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60533:
URL: https://github.com/apache/doris/pull/60533#issuecomment-3852519743
<details>
<summary>ClickBench: <b>Total hot run time: 28.41 s</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 86c0a22d28ad54ddd809f806891a3ac1e729508c, data reload: false
query1 0.06 0.05 0.05
query2 0.10 0.05 0.05
query3 0.25 0.08 0.08
query4 1.60 0.12 0.11
query5 0.26 0.24 0.25
query6 1.16 0.68 0.68
query7 0.03 0.03 0.03
query8 0.06 0.04 0.04
query9 0.57 0.51 0.49
query10 0.54 0.54 0.54
query11 0.13 0.09 0.09
query12 0.13 0.10 0.10
query13 0.64 0.61 0.62
query14 1.06 1.09 1.04
query15 0.88 0.86 0.86
query16 0.39 0.39 0.40
query17 1.10 1.12 1.15
query18 0.22 0.21 0.20
query19 2.10 1.95 1.95
query20 0.03 0.02 0.02
query21 15.43 0.28 0.15
query22 5.11 0.05 0.06
query23 15.95 0.30 0.11
query24 1.89 0.25 0.42
query25 0.08 0.11 0.09
query26 0.14 0.13 0.13
query27 0.07 0.07 0.05
query28 4.70 1.15 0.97
query29 12.55 3.92 3.16
query30 0.29 0.14 0.12
query31 2.82 0.64 0.42
query32 3.25 0.60 0.50
query33 3.23 3.31 3.31
query34 16.24 5.42 4.77
query35 4.82 4.78 4.81
query36 0.63 0.49 0.48
query37 0.12 0.07 0.07
query38 0.08 0.04 0.04
query39 0.05 0.03 0.03
query40 0.20 0.18 0.15
query41 0.10 0.03 0.03
query42 0.04 0.04 0.03
query43 0.05 0.04 0.03
Total cold run time: 99.15 s
Total hot run time: 28.41 s
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](filecache) avoid SIGSEGV in background LRU update when clear cache [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #60533:
URL: https://github.com/apache/doris/pull/60533#issuecomment-3852405109
<details>
<summary>TPC-H: <b>Total hot run time: 31410 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 86c0a22d28ad54ddd809f806891a3ac1e729508c, data reload: false
------ Round 1 ----------------------------------
q1 17658 4450 4362 4362
q2 2009 390 237 237
q3 10120 1321 721 721
q4 10217 923 338 338
q5 7560 2223 1974 1974
q6 195 180 147 147
q7 886 732 602 602
q8 9270 1441 1198 1198
q9 5280 4883 4840 4840
q10 6899 1975 1574 1574
q11 554 295 288 288
q12 381 388 227 227
q13 17783 4100 3206 3206
q14 235 240 214 214
q15 914 815 818 815
q16 685 686 634 634
q17 645 820 522 522
q18 7333 6638 6388 6388
q19 1119 1005 609 609
q20 392 366 254 254
q21 2725 2160 1986 1986
q22 372 320 274 274
Total cold run time: 103232 ms
Total hot run time: 31410 ms
----- Round 2, with runtime_filter_mode=off -----
q1 4353 4367 4342 4342
q2 277 344 261 261
q3 2140 2658 2226 2226
q4 1390 1752 1282 1282
q5 4310 4211 4225 4211
q6 219 179 137 137
q7 1827 1816 2025 1816
q8 2736 2491 2471 2471
q9 7548 7461 7454 7454
q10 2994 3307 2582 2582
q11 539 474 454 454
q12 684 765 615 615
q13 3987 4366 3581 3581
q14 285 323 284 284
q15 864 810 806 806
q16 682 733 693 693
q17 1140 1303 1344 1303
q18 8076 8003 8047 8003
q19 893 878 856 856
q20 2186 2162 2001 2001
q21 4934 4572 4418 4418
q22 602 577 525 525
Total cold run time: 52666 ms
Total hot run time: 50321 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](filecache) avoid SIGSEGV in background LRU update when clear cache [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60533:
URL: https://github.com/apache/doris/pull/60533#issuecomment-3878729869
# BE Regression && UT Coverage Report
Increment line coverage `100.00% (14/14)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60533_86c0a22d28ad54ddd809f806891a3ac1e729508c_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60533_86c0a22d28ad54ddd809f806891a3ac1e729508c_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 71.58% (25859/36124) |
| Line Coverage | 54.23% (270120/498085) |
| Region Coverage | 51.69% (224784/434878) |
| Branch Coverage | 53.14% (96487/181570) |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](filecache) avoid SIGSEGV in background LRU update when clear cache [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #60533:
URL: https://github.com/apache/doris/pull/60533#issuecomment-4010127793
## Code Review Summary
**PR #60533: [fix](filecache) avoid SIGSEGV in background LRU update when clear cache**
### Overview
This PR fixes a use-after-free / dangling pointer crash in the background LRU update thread. The root cause: `update_block_lru()` previously dereferenced `block->cell` — a raw pointer back into the `_files` map — which becomes dangling when `clear_file_cache_directly()` destroys all cells.
### Review Checklist
**Correctness**: PASS
The fix replaces direct `block->cell` dereference with a safe `get_cell()` lookup into `_files` under the cache lock. If the cell was removed, `get_cell()` returns `nullptr` and the function returns early. The identity check (`cell->file_block.get() != block.get()`) correctly guards against ABA-style reuse of the same hash+offset slot by a different block.
**Thread Safety**: PASS
`update_block_lru()` is called only from `run_background_block_lru_update()`, which holds `_mutex` via `SCOPED_CACHE_LOCK`. Both cache clearing paths (`clear_file_cache_async` and `clear_file_cache_directly`) also hold `_mutex`. The `get_cell()` lookup is therefore safe under the same lock.
**Initialization**: PASS
`FileBlockCell* cell` was previously uninitialized (`FileBlockCell* cell;`) — reading it before assignment was undefined behavior. The `{nullptr}` initializer is the correct fix.
**Behavioral Change**: Minor, acceptable
`cell->update_atime()` is now called even when `cell->queue_iterator` is not set (but only after confirming the cell exists and matches). Previously it was inside the `if (cell)` block but also conditional on cell existing. This is a benign change — updating atime even without a queue iterator is reasonable.
**Test Coverage**: PASS
The new test covers both the null-block path and the stale/corrupted cell pointer path. The test directly accesses `mgr._mutex` (via friend relationship), which is acceptable for this kind of internal unit test.
**No issues found.** The fix is well-targeted and correct.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](filecache) avoid SIGSEGV in background LRU update when clear cache [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #60533:
URL: https://github.com/apache/doris/pull/60533#issuecomment-4010100833
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](filecache) avoid SIGSEGV in background LRU update when clear cache [doris]
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on PR #60533:
URL: https://github.com/apache/doris/pull/60533#issuecomment-3851699215
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](filecache) avoid SIGSEGV in background LRU update when clear cache [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #60533:
URL: https://github.com/apache/doris/pull/60533#issuecomment-4010100974
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](filecache) avoid SIGSEGV in background LRU update when clear cache [doris]
Posted by "dataroaring (via GitHub)" <gi...@apache.org>.
dataroaring commented on PR #60533:
URL: https://github.com/apache/doris/pull/60533#issuecomment-4010108540
/review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org