WebGPUInProcessCommandBufferTest.Dummy crashes on first run |
|||||
Issue descriptiongl_tests:WebGPUInProcessCommandBufferTest.Dummy crashes on its first run, only passing when retried in a new test process: [ RUN ] WebGPUInProcessCommandBufferTest.Dummy [21925:21926:1020/160455.742634:24146468959:ERROR:in_process_command_buffer.cc(540)] ContextResult::kFatalFailure: WebGPU not enabled [21925:21925:1020/160455.742674:24146469000:ERROR:webgpu_in_process_context.cc(67)] Failed to initialize InProcessCommmandBuffer [21925:21925:1020/160455.742695:24146469019:FATAL:webgpu_in_process_context_tests.cc(38)] Check failed: result == ContextResult::kSuccess (2 vs. 0) #0 0x00000092cf9f base::debug::StackTrace::StackTrace() #1 0x0000008babdb logging::LogMessage::~LogMessage() #2 0x00000058bc9e gpu::(anonymous namespace)::WebGPUInProcessCommandBufferTest::SetUp() #3 0x0000008688ed testing::Test::Run() #4 0x000000869470 testing::TestInfo::Run() #5 0x000000869987 testing::TestCase::Run() #6 0x000000875ba7 testing::internal::UnitTestImpl::RunAllTests() #7 0x00000087571d testing::UnitTest::Run() #8 0x00000093c141 base::TestSuite::Run() #9 0x00000057642f (anonymous namespace)::RunHelper() #10 0x00000093ec9d base::(anonymous namespace)::LaunchUnitTestsInternal() #11 0x00000093f2e0 base::LaunchUnitTestsSerially() #12 0x0000005762a4 main #13 0x7f192496eec5 __libc_start_main #14 0x0000004fe02a _start The issue can be reproduced by running gl_tests with: --gtest_filter=GLInProcessCommandBufferTest.CreateImage:WebGPUInProcessCommandBufferTest.Dummy suggesting that the GLInProcessCommandBufferTest leaks state which breaks the later test.
,
Oct 22
Both WebGPUInProcessCommandBufferTest and GLInProcessCommandBufferTest are using the default constructed GpuInProcessThreadHolder which is a global static. WebGPUInProcessCommandBufferTest should provide a CommandBufferTaskExecutor implementation that supports webgpu when it creates the GLInProcessContext instead of nullptr.
,
Oct 22
When this gets resolved, you might add a post-test check that this global state got leaked by the test, e.g. see https://cs.chromium.org/chromium/src/base/test/test_suite.cc?l=110
,
Oct 22
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/be3c82ec7012add86fe73c1d9b6684b7700db673 commit be3c82ec7012add86fe73c1d9b6684b7700db673 Author: Wez <wez@chromium.org> Date: Mon Oct 22 17:53:13 2018 Disable GLInProcessCommandBufferTest.CreateImage test. This test leaks process-global state, causing potentially causing later tests (e.g. WebGPUInProcessCommandBufferTest.Dummy) run in the same batch process to unexpectedly fail. Bug: 897456 , 879806 Change-Id: Ied7facc983f7f62e6954e3d3ad28572010b09ae5 Reviewed-on: https://chromium-review.googlesource.com/c/1293030 Reviewed-by: Eric Karl <ericrk@chromium.org> Commit-Queue: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#601640} [modify] https://crrev.com/be3c82ec7012add86fe73c1d9b6684b7700db673/gpu/ipc/client/gpu_in_process_context_tests.cc
,
Oct 22
The whole concept of a default constructed global GpuInProcessThreadHolder that has state based on command line flags is problematic. I'll take a look at getting rid of GpuInProcessThreadHolder and having all tests pass in a CommandBufferTaskExecutor.
,
Oct 23
+a few gpu/OWNERS I'm looking at the current GpuInProcessThreadHolder implementation in InProcessCommandBuffer. If a null CommandBufferTaskExecutor is passed in to the constructor then InProcessCommandBuffer gets a CommandBufferTaskExecutor from a global GpuInProcessThreadHolder. The first time this happens the global GpuInProcessThreadHolder is initialized, starting a thread for the GPU, and then a CommandBufferTaskExecutor is initialized on the GPU thread. The GpuFeatureInfo and GpuPreferences are fixed after the CommandBufferTaskExecutor is initialized. There is the option to set the GpuFeatureInfo to something specific before CommandBufferTaskExecutor is initialized. Having this default thread/task executor makes using InProcessCommandBuffer in tests a bit more convenient but the state is dependent on what test first uses it. Also, being able to accidentally use the global GpuInProcessThreadHolder and it's CommandBufferTaskExecutor in production code seems like a dangerous thing. I'd propose doing the following: 1. Moving GpuInProcessThreadHolder into it's own class and out of InProcessCommandBuffer. 2. Moving the global GpuInProcessThreadHolder out of InProcessCommandBuffer entirely and into some other file. 3. For test targets that want to use the global GpuInProcessThreadHolder they'll have to opt-in explicitly. This looks like cc_unittests, compositor_unittests, content_unittests and viz_unittests. 3. Have GLInProcessCommandBufferTest, WebGPUInProcessCommandBufferTest and RasterInProcessCommandBufferTest create their own GpuInProcessThreadHolder instead of using the global. 4. Re-enabling the disabled test. Any thoughts?
,
Oct 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d6d589300fb1898becd2103568ff72b76013b1d5 commit d6d589300fb1898becd2103568ff72b76013b1d5 Author: kylechar <kylechar@chromium.org> Date: Fri Oct 26 17:57:22 2018 Fix global state leak in gl_tests. There is a global CommandBufferTaskExecutor that gets created if an instance isn't provided. This task executor picks up the current command line to create GpuPreferences on first use, which caused problems where depending on what test ran first the GpuPreferences would be different and later tests would fail. Don't use the global task executor in gl_tests. Move and rename InProcessGpuThreadHolder so that each test which needs it can instantiate the object itself. Also enable GLInProcessCommandBufferTest.CreateImage again. Bug: 897456 Change-Id: Icdc307bd68453fdc2b898a9765a3f903b21fdfe8 Reviewed-on: https://chromium-review.googlesource.com/c/1298214 Commit-Queue: kylechar <kylechar@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#603143} [modify] https://crrev.com/d6d589300fb1898becd2103568ff72b76013b1d5/gpu/command_buffer/tests/gl_tests_main.cc [modify] https://crrev.com/d6d589300fb1898becd2103568ff72b76013b1d5/gpu/ipc/BUILD.gn [modify] https://crrev.com/d6d589300fb1898becd2103568ff72b76013b1d5/gpu/ipc/client/gpu_in_process_context_tests.cc [modify] https://crrev.com/d6d589300fb1898becd2103568ff72b76013b1d5/gpu/ipc/client/raster_in_process_context_tests.cc [modify] https://crrev.com/d6d589300fb1898becd2103568ff72b76013b1d5/gpu/ipc/client/webgpu_in_process_context_tests.cc [modify] https://crrev.com/d6d589300fb1898becd2103568ff72b76013b1d5/gpu/ipc/in_process_command_buffer.cc [add] https://crrev.com/d6d589300fb1898becd2103568ff72b76013b1d5/gpu/ipc/in_process_gpu_thread_holder.cc [add] https://crrev.com/d6d589300fb1898becd2103568ff72b76013b1d5/gpu/ipc/in_process_gpu_thread_holder.h
,
Nov 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1437abb2dd74bfbc10972a58a67775dba800b5ab commit 1437abb2dd74bfbc10972a58a67775dba800b5ab Author: kylechar <kylechar@chromium.org> Date: Tue Nov 06 20:36:08 2018 Make InProcessGpuThreadHolder test only. InProcessGpuThreadHolder is only used with tests so make it test only. Move the global instance out InProcessCommandBuffer to make this possible. Note the global is in a component to ensure that we don't end up with multiple copies of it in different shared libraries. Bug: 897456 Change-Id: I5155529c9386b3af53b1e610106d2ceb6adb87f5 Reviewed-on: https://chromium-review.googlesource.com/c/1317707 Reviewed-by: Antoine Labour <piman@chromium.org> Reviewed-by: Christopher Grant <cjgrant@chromium.org> Commit-Queue: kylechar <kylechar@chromium.org> Cr-Commit-Position: refs/heads/master@{#605809} [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/cc/BUILD.gn [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/cc/paint/transfer_cache_unittest.cc [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/cc/test/cc_test_suite.cc [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/cc/test/test_in_process_context_provider.cc [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/chrome/browser/vr/test/gl_test_environment_cmd_buffer.cc [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/chrome/browser/vr/test/vr_gl_test_suite.cc [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/components/viz/common/BUILD.gn [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/components/viz/common/DEPS [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/components/viz/common/gl_helper_benchmark.cc [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/components/viz/common/gl_helper_unittest.cc [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/components/viz/common/yuv_readback_unittest.cc [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/content/test/content_test_suite.cc [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/gpu/BUILD.gn [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/gpu/ipc/BUILD.gn [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/gpu/ipc/in_process_command_buffer.cc [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/gpu/ipc/in_process_command_buffer.h [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/gpu/ipc/in_process_gpu_thread_holder.cc [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/gpu/ipc/in_process_gpu_thread_holder.h [add] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/gpu/ipc/test_gpu_thread_holder.cc [add] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/gpu/ipc/test_gpu_thread_holder.h [modify] https://crrev.com/1437abb2dd74bfbc10972a58a67775dba800b5ab/ui/compositor/test/in_process_context_provider.cc
,
Nov 6
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by w...@chromium.org
, Oct 20