New issue
Advanced search Search tips

Issue 850822 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

JpegDecodeAccelerator unit tests can call the decoder's destructor in the wrong thread

Project Member Reported by andrescj@chromium.org, Jun 8 2018

Issue description

In a normal scenario, the JpegDecodeAcceleratorTest::TestDecode() method in media/gpu/jpeg_decode_accelerator_unittest.cc starts a new thread, creates a JpegClient, and then posts tasks to the new thread to have the JpegClient create a decoder, invoke its functionality, and destroy it.

However, if, e.g., an ASSERT_EQ fails, the JpegClient instance is destroyed in the main thread, causing its decoder_ member to be destroyed in the main thread as well. This is incorrect because [1] and [2] require that the destructors be called in the thread in which the objects were created.

[1] https://cs.chromium.org/chromium/src/media/gpu/vaapi/vaapi_jpeg_decode_accelerator.cc?l=126
[2] https://cs.chromium.org/chromium/src/media/gpu/v4l2/v4l2_jpeg_decode_accelerator.cc?dr=CSs&g=0&l=160
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 8 2018

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

commit 2d0e75a06d24ff78139649b12b245e447f62f518
Author: Andres Calderon Jaramillo <andrescj@chromium.org>
Date: Fri Jun 08 23:01:08 2018

JDA unit tests: call the JDA destructor in the right thread.

This CL modifies the JDA unit tests to call the JpegDecodeAccelerator
destructor in the correct thread when the JpegClient gets destroyed.

The strategy is to destroy the JpegClient in the thread in which the
JpegDecodeAccelerator is created. To do so, we use a ScopedClosureRunner
which posts a task to the appropriate thread to delete the JpegClient
instance. This causes its decoder_ member to be deleted in that thread
as well. We must be careful with the order of destruction though: the
JpegClient cannot be destroyed until all the tasks posted by decoder_ in
a different thread spawned by decoder_ have finished. The destructor of
decoder_ should not exit until that happens. Also, notice that now,
JpegClient takes ownership of the ClientStateNotification passed in its
constructor. This is because we can't let the ClientStateNotification
die before decoder_: for example, suppose we post a task to delete the
JpegClient as described above. It is still possible that there are
pending tasks created by decoder_ that require the use of the
ClientStateNotification instance.

Bug:  850822 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I9c2b16676e903fb229232bec9e4b25fe0a74f724
Reviewed-on: https://chromium-review.googlesource.com/1091598
Commit-Queue: Andres Calderon Jaramillo <andrescj@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565768}
[modify] https://crrev.com/2d0e75a06d24ff78139649b12b245e447f62f518/media/gpu/jpeg_decode_accelerator_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment