New issue
Advanced search Search tips

Issue 760063 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

[Windows] Crash on fallback from ANGLE to SwiftShader.

Project Member Reported by mpichlin...@opera.com, Aug 29 2017

Issue description

Chrome Version: 63.0.3188.4
OS: Windows

What steps will reproduce the problem?
(1) Run gl_unittests.exe on Windows Machine which does not support hardware GL

What is the expected result?
Tests should pass.

What happens instead?
Tests crash during fallback from ANGLE to SwiftShader.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/21328491476dcf313bd0d68387edf378aad8c19c

commit 21328491476dcf313bd0d68387edf378aad8c19c
Author: Michał Pichliński <mpichlinski@opera.com>
Date: Tue Sep 05 01:20:28 2017

CHR-6375: [Windows] Fixed crash on fallback from ANGLE to SwiftShader.

During fallback from ANGLE to SwiftShader it is required to unload
ANGLE libraries, otherwise SwiftShader will fail to load its own
libGLESv2.dll.

Fixed ANGLE platform reset.

Bug:  760063 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I02280b2c1cd6c3d81087c7e2befb412bb2a89510
Reviewed-on: https://chromium-review.googlesource.com/640992
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499529}
[modify] https://crrev.com/21328491476dcf313bd0d68387edf378aad8c19c/ui/gl/angle_platform_impl.cc
[modify] https://crrev.com/21328491476dcf313bd0d68387edf378aad8c19c/ui/gl/gl_implementation.cc

Status: Fixed (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4bf7e3c60ca30098a13cc9659514496bee9fc555

commit 4bf7e3c60ca30098a13cc9659514496bee9fc555
Author: Christos Froussios <cfroussios@chromium.org>
Date: Tue Sep 05 08:27:22 2017

Revert "CHR-6375: [Windows] Fixed crash on fallback from ANGLE to SwiftShader."

This reverts commit 21328491476dcf313bd0d68387edf378aad8c19c.

Reason for revert: Suspected of breaking GpuHostTest.GpuClientDestructionOrder
on Builder: Linux Chromium OS ASan LSan Tests
 crbug.com/761930 

Original change's description:
> CHR-6375: [Windows] Fixed crash on fallback from ANGLE to SwiftShader.
> 
> During fallback from ANGLE to SwiftShader it is required to unload
> ANGLE libraries, otherwise SwiftShader will fail to load its own
> libGLESv2.dll.
> 
> Fixed ANGLE platform reset.
> 
> Bug:  760063 
> Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
> Change-Id: I02280b2c1cd6c3d81087c7e2befb412bb2a89510
> Reviewed-on: https://chromium-review.googlesource.com/640992
> Commit-Queue: Jamie Madill <jmadill@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Reviewed-by: Jamie Madill <jmadill@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#499529}

TBR=jmadill@chromium.org,piman@chromium.org,mpichlinski@opera.com

Change-Id: I6d00ee495a9f4efdf03dd6be8aa15426241548a1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  760063 , 761930 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/650286
Reviewed-by: Christos Froussios <cfroussios@chromium.org>
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499583}
[modify] https://crrev.com/4bf7e3c60ca30098a13cc9659514496bee9fc555/ui/gl/angle_platform_impl.cc
[modify] https://crrev.com/4bf7e3c60ca30098a13cc9659514496bee9fc555/ui/gl/gl_implementation.cc

Status: Assigned (was: Fixed)
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9fe0ca099575f8dd90444bcadbb409007e9a547b

commit 9fe0ca099575f8dd90444bcadbb409007e9a547b
Author: Michał Pichliński <mpichlinski@opera.com>
Date: Fri Sep 15 22:32:42 2017

CHR-6375: Fixed crash on fallback from ANGLE to SwiftShader.

During fallback from ANGLE to SwiftShader it is required to unload
ANGLE libraries, otherwise SwiftShader will fail to load its own
libGLESv2 library.

Fixed ANGLE platform reset.

Fixed memory leak on X11 by fixing order in ShutdownGL.

Leak was occuring during fallback from libGL to software GL implementation
because ShutdownGL was cleaning GL implementation info before unloading
GL and therefore libGL was unloaded with known issue crbug.com/250813

Previous attempt with memory leak:
https://chromium-review.googlesource.com/640992

Reverted in:
https://chromium-review.googlesource.com/650286

Bug:  760063 ,  761930 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ibea80f560aa50ba48cbff6f39a664095db38daaf
Reviewed-on: https://chromium-review.googlesource.com/668357
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502409}
[modify] https://crrev.com/9fe0ca099575f8dd90444bcadbb409007e9a547b/ui/gl/angle_platform_impl.cc
[modify] https://crrev.com/9fe0ca099575f8dd90444bcadbb409007e9a547b/ui/gl/gl_implementation.cc
[modify] https://crrev.com/9fe0ca099575f8dd90444bcadbb409007e9a547b/ui/gl/init/gl_factory.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6864b6e6f970dae48adc5131f3ca1204b7ea2475

commit 6864b6e6f970dae48adc5131f3ca1204b7ea2475
Author: Max Morin <maxmorin@chromium.org>
Date: Mon Sep 18 11:16:26 2017

Revert "CHR-6375: Fixed crash on fallback from ANGLE to SwiftShader."

This reverts commit 9fe0ca099575f8dd90444bcadbb409007e9a547b.

Reason for revert: Still breaking GpuHostTest.GpuClientDestructionOrder
on Builder: Linux Chromium OS ASan LSan Tests.
https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/23657:
[       OK ] GpuHostTest.GpuClientDestructionOrder (100 ms)
[----------] 1 test from GpuHostTest (100 ms total)
[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (100 ms total)
[  PASSED  ] 1 test.
=================================================================
==31215==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0x540cf3 in __interceptor_malloc (/b/s/w/ir/out/Release/services_unittests+0x540cf3)
    #1 0x7f979021987f in XextAddDisplay src/extutil.c:106
Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x540cf3 in __interceptor_malloc (/b/s/w/ir/out/Release/services_unittests+0x540cf3)
    #1 0x7f979021980d in XextCreateExtension src/extutil.c:68
    #2 0x621000030cff  (<unknown module>)
Indirect leak of 67200 byte(s) in 300 object(s) allocated from:
    #0 0x540cf3 in __interceptor_malloc (/b/s/w/ir/out/Release/services_unittests+0x540cf3)
    #1 0x7f97835ca364  (<unknown module>)
Indirect leak of 8288 byte(s) in 2 object(s) allocated from:
    #0 0x540cf3 in __interceptor_malloc (/b/s/w/ir/out/Release/services_unittests+0x540cf3)
    #1 0x7f97835ccc2d  (<unknown module>)
Indirect leak of 299 byte(s) in 1 object(s) allocated from:
    #0 0x540cf3 in __interceptor_malloc (/b/s/w/ir/out/Release/services_unittests+0x540cf3)
    #1 0x7f97835aaae3  (<unknown module>)
Indirect leak of 249 byte(s) in 3 object(s) allocated from:
    #0 0x540cf3 in __interceptor_malloc (/b/s/w/ir/out/Release/services_unittests+0x540cf3)
    #1 0x7f97835c8f7f  (<unknown module>)
Indirect leak of 224 byte(s) in 1 object(s) allocated from:
    #0 0x540eea in __interceptor_calloc (/b/s/w/ir/out/Release/services_unittests+0x540eea)
    #1 0x7f97835c96e6  (<unknown module>)
Indirect leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x540eea in __interceptor_calloc (/b/s/w/ir/out/Release/services_unittests+0x540eea)
    #1 0x7f97835aa331  (<unknown module>)
Indirect leak of 104 byte(s) in 1 object(s) allocated from:
    #0 0x540cf3 in __interceptor_malloc (/b/s/w/ir/out/Release/services_unittests+0x540cf3)
    #1 0x7f97835d1b51  (<unknown module>)
Indirect leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0x540cf3 in __interceptor_malloc (/b/s/w/ir/out/Release/services_unittests+0x540cf3)
    #1 0x7f979021987f in XextAddDisplay src/extutil.c:106
Indirect leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x540cf3 in __interceptor_malloc (/b/s/w/ir/out/Release/services_unittests+0x540cf3)
    #1 0x7f97835c9d1d  (<unknown module>)
Indirect leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x540cf3 in __interceptor_malloc (/b/s/w/ir/out/Release/services_unittests+0x540cf3)
    #1 0x7f97835aa4b3  (<unknown module>)
-----------------------------------------------------
Suppressions used:
  count      bytes template
    123      22299 swrast_dri.so
-----------------------------------------------------
SUMMARY: AddressSanitizer: 76588 byte(s) leaked in 314 allocation(s).

Original change's description:
> CHR-6375: Fixed crash on fallback from ANGLE to SwiftShader.
> 
> During fallback from ANGLE to SwiftShader it is required to unload
> ANGLE libraries, otherwise SwiftShader will fail to load its own
> libGLESv2 library.
> 
> Fixed ANGLE platform reset.
> 
> Fixed memory leak on X11 by fixing order in ShutdownGL.
> 
> Leak was occuring during fallback from libGL to software GL implementation
> because ShutdownGL was cleaning GL implementation info before unloading
> GL and therefore libGL was unloaded with known issue crbug.com/250813
> 
> Previous attempt with memory leak:
> https://chromium-review.googlesource.com/640992
> 
> Reverted in:
> https://chromium-review.googlesource.com/650286
> 
> Bug:  760063 ,  761930 
> Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
> Change-Id: Ibea80f560aa50ba48cbff6f39a664095db38daaf
> Reviewed-on: https://chromium-review.googlesource.com/668357
> Reviewed-by: Jamie Madill <jmadill@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Commit-Queue: Jamie Madill <jmadill@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#502409}

TBR=jmadill@chromium.org,piman@chromium.org,mpichlinski@opera.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  760063 ,  761930 
Change-Id: I3e286b7b7ce6aa97ab2b8e412f490ad4498d1f7b
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/670705
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502551}
[modify] https://crrev.com/6864b6e6f970dae48adc5131f3ca1204b7ea2475/ui/gl/angle_platform_impl.cc
[modify] https://crrev.com/6864b6e6f970dae48adc5131f3ca1204b7ea2475/ui/gl/gl_implementation.cc
[modify] https://crrev.com/6864b6e6f970dae48adc5131f3ca1204b7ea2475/ui/gl/init/gl_factory.cc

Cc: jmad...@chromium.org
Hi Jamie!

In https://chromium-review.googlesource.com/c/chromium/src/+/640992/1#message-22873c8857c5b55599f671db25521812db1da573 you asked me to enable fix for linux. However on asan builders services_unittests fails due to memory leaks. Probably this is the reason of disabling libraries unload (I have no access to the crbug.com/250813 so this is my guess). Unfortunately I cannot reproduce memory leaks, which occurs on builders. Can I fix it for windows only?

Best regards,
Michał
Cc: piman@chromium.org
I don't really have a problem with that, but it's a bit confusing to me what's happening in the code. Can you work with piman@ to land your change?
Hi Piman!

I can limit scope of the leak by unloading libraries only in the case of a fallback from the swiftshader to the software rendering, by extending ShutdownGL, UnloadGLNativeLibraries and CleanupNativeLibraries with a parameter due_to_fallback and check whether due_to_fallback is true and g_gl_implementation is kGLImplementationSwiftShaderGL.

In my opinion it is better to leak some memory than crash (I dunno how often gl is being shutdown). Maybe fallback case does not occur during tests run on asan builder, but if it does we could suppress this particular error.

Are you ok with that solution?

Best regards,
Michał
mpichlinski, you can always throw up a review and add us as reviewers.

Comment 11 by piman@chromium.org, Nov 16 2017

crbug.com/250813 is about libva on Chrome OS things, and was a bigger deal than leaks, but Chrome OS doesn't use X any more, so it's a bit irrelevant. That said, the comment is still true that unloading libGL without closing the display might cause trouble (beyond leaks).

Is it true that we need to unload GL libraries on linux though? AFAIK we dlopen without RTLD_GLOBAL, so the different libraries should be able to coexist.
Project Member

Comment 12 by bugdroid1@chromium.org, Nov 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aeb6e40730e8c5cf3888b44609502da39885e02a

commit aeb6e40730e8c5cf3888b44609502da39885e02a
Author: Michał Pichliński <mpichlinski@opera.com>
Date: Thu Nov 30 22:52:43 2017

CHR-6375: [Windows] Fixed crash on fallback from ANGLE to SwiftShader.

During fallback from ANGLE to SwiftShader it is required to unload
ANGLE libraries, otherwise SwiftShader will fail to load its own
libGLESv2.dll.

Fixed ANGLE platform reset.

Fixed order in ShutdownGL.

Bug:  760063 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: If8797b49f2b5d04cb610302f96c82011dee9b3a9
Reviewed-on: https://chromium-review.googlesource.com/774296
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520730}
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/gpu/command_buffer/service/gpu_service_test.cc
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/gpu/config/gpu_info_collector_unittest.cc
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/gpu/ipc/client/gpu_memory_buffer_impl_test_template.h
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/gpu/ipc/service/gpu_channel_test_common.cc
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/gpu/ipc/service/gpu_memory_buffer_factory_test_template.h
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/media/gpu/android/android_video_decode_accelerator_unittest.cc
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/media/gpu/android/codec_image_unittest.cc
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/media/gpu/android/surface_texture_gl_owner_unittest.cc
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/services/ui/ws/gpu_host_unittest.cc
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/ui/gl/angle_platform_impl.cc
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/ui/gl/gl_implementation.cc
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/ui/gl/gl_implementation.h
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/ui/gl/gpu_timing_unittest.cc
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/ui/gl/init/gl_factory.cc
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/ui/gl/init/gl_factory.h
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/ui/gl/test/gl_image_test_support.cc
[modify] https://crrev.com/aeb6e40730e8c5cf3888b44609502da39885e02a/ui/gl/test/gl_surface_test_support.cc

Status: Fixed (was: Assigned)

Sign in to add a comment