[fix] _cur_reader can be null in exception cases in VFileScanner #40273
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
|
run buildall |
Vallishp
force-pushed
the
file_scanner_null_ptr
branch
from
6d2a1d1
to
46cb996
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // KIND, either express or implied. See the License for the | ||
| // specific language governing permissions and limitations | ||
| // under the License. | ||
| #include <gtest/gtest.h> |
There was a problem hiding this comment.
warning: 'gtest/gtest.h' file not found [clang-diagnostic-error]
#include <gtest/gtest.h>
^
Comment on lines
+35
to
+37
| namespace doris { | ||
|
|
||
| namespace vectorized { |
There was a problem hiding this comment.
warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
Suggested change
| namespace doris { | |
| namespace vectorized { | |
| namespace doris::vectorized { |
be/test/vec/exec/vfile_scanner_exception_test.cpp:304:
- } // namespace vectorized
- } // namespace doris
+ } // namespace doris| WARN_IF_ERROR(_scan_node->close(&_runtime_state), "fail to close scan_node") | ||
| } | ||
|
|
||
| protected: |
There was a problem hiding this comment.
warning: 'virtual' is redundant since the function is already declared 'override' [modernize-use-override]
Suggested change
| protected: | |
| void SetUp() override {} |
| std::unique_ptr<ShardedKVCache> _kv_cache = nullptr; | ||
| std::unique_ptr<TMasterInfo> _master_info = nullptr; | ||
| }; | ||
|
|
There was a problem hiding this comment.
warning: function '_init_desc_table' exceeds recommended size/complexity thresholds [readability-function-size]
void VfileScannerExceptionTest::_init_desc_table() {
^Additional context
be/test/vec/exec/vfile_scanner_exception_test.cpp:111: 117 lines including whitespace and comments (threshold 80)
void VfileScannerExceptionTest::_init_desc_table() {
^
Comment on lines
+35
to
+37
| namespace doris { | ||
|
|
||
| namespace vectorized { |
There was a problem hiding this comment.
warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
Suggested change
| namespace doris { | |
| namespace vectorized { | |
| namespace doris::vectorized { |
be/test/vec/exec/vfile_scanner_exception_test.cpp:305:
- } // namespace vectorized
- } // namespace doris
+ } // namespace doris| } | ||
|
|
||
| protected: | ||
| virtual void SetUp() override {} |
There was a problem hiding this comment.
warning: 'virtual' is redundant since the function is already declared 'override' [modernize-use-override]
Suggested change
| virtual void SetUp() override {} | |
| void SetUp() override {} |
| std::unique_ptr<TMasterInfo> _master_info = nullptr; | ||
| }; | ||
|
|
||
| void VfileScannerExceptionTest::_init_desc_table() { |
There was a problem hiding this comment.
warning: function '_init_desc_table' exceeds recommended size/complexity thresholds [readability-function-size]
void VfileScannerExceptionTest::_init_desc_table() {
^Additional context
be/test/vec/exec/vfile_scanner_exception_test.cpp:112: 117 lines including whitespace and comments (threshold 80)
void VfileScannerExceptionTest::_init_desc_table() {
^
github-actions
bot
added
the
approved
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
yiguolei
previously approved these changes
Sep 10, 2024
|
run buildall |
yiguolei
force-pushed
the
file_scanner_null_ptr
branch
from
32efeca
to
1266aa8
Compare
|
run buildall |
TPC-H: Total hot run time: 38384 ms |
TPC-DS: Total hot run time: 197232 ms |
ClickBench: Total hot run time: 32 s |
|
run buildall |
TPC-H: Total hot run time: 38454 ms |
TPC-DS: Total hot run time: 198301 ms |
ClickBench: Total hot run time: 31.83 s |
Vallishp
force-pushed
the
file_scanner_null_ptr
branch
from
caf8722
to
eee77b5
Compare
|
run buildall |
github-actions
bot
removed
the
approved
|
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 38182 ms |
TPC-DS: Total hot run time: 196890 ms |
ClickBench: Total hot run time: 32.08 s |
yiguolei
approved these changes
Sep 11, 2024
github-actions
bot
added
the
approved
|
PR approved by at least one committer and no changes requested. |
|
run buildall |
|
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 38566 ms |
TPC-DS: Total hot run time: 199066 ms |
ClickBench: Total hot run time: 31.24 s |
dataroaring
pushed a commit
that referenced
this pull request
Oct 9, 2024
) ## Proposed changes Issue Number: close #xxx cur_reader pointer can be null in VFileScanner. can cause BE crash [ RUN ] VfileScannerExcepTest.failure_case AddressSanitizer:DEADLYSIGNAL ================================================================= ==1892247==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x5cdbe26eb5f8 bp 0x7ffed6728610 sp 0x7ffed67277a0 T0) ==1892247==The signal is caused by a READ memory access. ==1892247==Hint: address points to the zero page. #0 0x5cdbe26eb5f8 in doris::vectorized::VFileScanner::_get_next_reader() /root/doris/workspace/doris/be/src/vec/exec/scan/vfile_scanner.cpp:980:9 #1 0x5cdbe26e3512 in doris::vectorized::VFileScanner::_get_block_wrapped(doris::RuntimeState*, doris::vectorized::Block*, bool*) /root/doris/workspace/doris/be/src/vec/exec/scan/vfile_scanner.cpp:286:25 #2 0x5cdbe26e2e46 in doris::vectorized::VFileScanner::_get_block_impl(doris::RuntimeState*, doris::vectorized::Block*, bool*) /root/doris/workspace/doris/be/src/vec/exec/scan/vfile_scanner.cpp:252:17 #3 0x5cdbe28e05ce in doris::vectorized::VScanner::get_block(doris::RuntimeState*, doris::vectorized::Block*, bool*) /root/doris/workspace/doris/be/src/vec/exec/scan/vscanner.cpp:117:17 #4 0x5cdbc405a922 in doris::vectorized::VfileScannerExcepTest_failure_case_Test::TestBody() /root/doris/workspace/doris/be/test/vec/exec/vfile_scanner_excep_test.cpp:309:24 #5 0x5cdbff5b191a in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/root/doris/workspace/doris/be/ut_build_ASAN/test/doris_be_test+0x51eb491a) (BuildId: 21d41a2d207823b9) #6 0x5cdbff59f989 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/root/doris/workspace/doris/be/ut_build_ASAN/test/doris_be_test+0x51ea2989) (BuildId: 21d41a2d207823b9) #7 0x5cdbff57a9c2 in testing::Test::Run() (/root/doris/workspace/doris/be/ut_build_ASAN/test/doris_be_test+0x51e7d9c2) (BuildId: 21d41a2d207823b9) #8 0x5cdbff57b708 in testing::TestInfo::Run() (/root/doris/workspace/doris/be/ut_build_ASAN/test/doris_be_test+0x51e7e708) (BuildId: 21d41a2d207823b9) #9 0x5cdbff57bec3 in testing::TestSuite::Run() (/root/doris/workspace/doris/be/ut_build_ASAN/test/doris_be_test+0x51e7eec3) (BuildId: 21d41a2d207823b9) After fix: I20240902 09:11:07.722273 1946048 run_all_tests.cpp:67] init config 1 Note: Google Test filter = VfileScannerE* [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from VfileScannerExceptionTest [ RUN ] VfileScannerExceptionTest.failure_case msg = [INTERNAL_ERROR]cur path: . Failed to create reader for file format: 11 [ OK ] VfileScannerExceptionTest.failure_case (3 ms) [----------] 1 test from VfileScannerExceptionTest (3 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test suite ran. (3 ms total) [ PASSED ] 1 test. === Finished. Gtest output: /root/doris/workspace/doris/be/ut_build_ASAN/gtest_output
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
Issue Number: close #xxx
cur_reader pointer can be null in VFileScanner. can cause BE crash
[ RUN ] VfileScannerExcepTest.failure_case
AddressSanitizer:DEADLYSIGNAL
==1892247==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x5cdbe26eb5f8 bp 0x7ffed6728610 sp 0x7ffed67277a0 T0)
==1892247==The signal is caused by a READ memory access.
==1892247==Hint: address points to the zero page.
#0 0x5cdbe26eb5f8 in doris::vectorized::VFileScanner::_get_next_reader() /root/doris/workspace/doris/be/src/vec/exec/scan/vfile_scanner.cpp:980:9
#1 0x5cdbe26e3512 in doris::vectorized::VFileScanner::_get_block_wrapped(doris::RuntimeState*, doris::vectorized::Block*, bool*) /root/doris/workspace/doris/be/src/vec/exec/scan/vfile_scanner.cpp:286:25
#2 0x5cdbe26e2e46 in doris::vectorized::VFileScanner::_get_block_impl(doris::RuntimeState*, doris::vectorized::Block*, bool*) /root/doris/workspace/doris/be/src/vec/exec/scan/vfile_scanner.cpp:252:17
#3 0x5cdbe28e05ce in doris::vectorized::VScanner::get_block(doris::RuntimeState*, doris::vectorized::Block*, bool*) /root/doris/workspace/doris/be/src/vec/exec/scan/vscanner.cpp:117:17
#4 0x5cdbc405a922 in doris::vectorized::VfileScannerExcepTest_failure_case_Test::TestBody() /root/doris/workspace/doris/be/test/vec/exec/vfile_scanner_excep_test.cpp:309:24
#5 0x5cdbff5b191a in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::)(), char const) (/root/doris/workspace/doris/be/ut_build_ASAN/test/doris_be_test+0x51eb491a) (BuildId: 21d41a2d207823b9)
#6 0x5cdbff59f989 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::)(), char const) (/root/doris/workspace/doris/be/ut_build_ASAN/test/doris_be_test+0x51ea2989) (BuildId: 21d41a2d207823b9)
#7 0x5cdbff57a9c2 in testing::Test::Run() (/root/doris/workspace/doris/be/ut_build_ASAN/test/doris_be_test+0x51e7d9c2) (BuildId: 21d41a2d207823b9)
#8 0x5cdbff57b708 in testing::TestInfo::Run() (/root/doris/workspace/doris/be/ut_build_ASAN/test/doris_be_test+0x51e7e708) (BuildId: 21d41a2d207823b9)
#9 0x5cdbff57bec3 in testing::TestSuite::Run() (/root/doris/workspace/doris/be/ut_build_ASAN/test/doris_be_test+0x51e7eec3) (BuildId: 21d41a2d207823b9)
After fix:
I20240902 09:11:07.722273 1946048 run_all_tests.cpp:67] init config 1
Note: Google Test filter = VfileScannerE*
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from VfileScannerExceptionTest
[ RUN ] VfileScannerExceptionTest.failure_case
msg = [INTERNAL_ERROR]cur path: . Failed to create reader for file format: 11
[ OK ] VfileScannerExceptionTest.failure_case (3 ms)
[----------] 1 test from VfileScannerExceptionTest (3 ms total)
[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (3 ms total)
[ PASSED ] 1 test.
=== Finished. Gtest output: /root/doris/workspace/doris/be/ut_build_ASAN/gtest_output