Race between PrefService and PrefMember deletion

[Marshall Greenblatt]

Hi All,

In my Chromium-based application I have multiple Profile objects that may be deleted before Chromium shuts down. I also have a BrowserMessageFilter that owns a PrefMember like so (see chrome/browser/printing/printing_message_filter.h for a concrete example):

class MyMessageFilter : public BrowserMessageFilter {

 public:

  MyMessageFilter(Profile* profile) {

    pref_->Init(prefs::kMyPrefName, profile->GetPrefs());

    pref_->MoveToThread(

        BrowserThread::GetTaskRunnerForThread(BrowserThread::IO));

  }

  // MyMessageFilter does something with |pref_| on the IO thread.

 private:

   std::unique_ptr<BooleanPrefMember, BrowserThread::DeleteOnUIThread> pref_;

};

// Create and install MyMessageFilter.

MyContentBrowserClient::RenderProcessWillLaunch(RenderProcessHost* host) {

  Profile* profile = Profile::FromBrowserContext(host->GetBrowserContext());

  host->AddFilter(new MyMessageFilter(profile));

}

The problem is that my Profile object (and the PrefService that it owns) may be deleted on the UI thread before the PrefMember object is deleted on the UI thread due to the async execution of DeleteOnUIThread. This results in a heap-use-after-free when the PrefMember is deleted due to it referencing the (already deleted) PrefService.

It seems like I need to do one of the following:

A. Disconnect all PrefMembers when the PrefService is deleted, or;

B. Delay deletion of the PrefService until all PrefMembers have disconnected.

Is there a recommended way to handle this currently?

Thanks,

Marshall

[Bernhard Bauer]

A is the right way to go. The PrefMember shouldn't have DeleteOnUIThread in the first place, because if it is correctly detached when the PrefService goes away, it can be destroyed on any thread. 

Bernhard.

 

Thanks,

Marshall

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

[Marshall Greenblatt]

On Nov 24, 2016, at 6:46 AM, Bernhard Bauer <bau...@chromium.org> wrote:

On Thu, Nov 24, 2016 at 3:22 AM Marshall Greenblatt <magree...@gmail.com> wrote:

Hi All,

In my Chromium-based application I have multiple Profile objects that may be deleted before Chromium shuts down. I also have a BrowserMessageFilter that owns a PrefMember like so (see chrome/browser/printing/printing_message_filter.h for a concrete example):

class MyMessageFilter : public BrowserMessageFilter {

 public:

  MyMessageFilter(Profile* profile) {

    pref_->Init(prefs::kMyPrefName, profile->GetPrefs());

    pref_->MoveToThread(

        BrowserThread::GetTaskRunnerForThread(BrowserThread::IO));

  }

  // MyMessageFilter does something with |pref_| on the IO thread.

 private:

   std::unique_ptr<BooleanPrefMember, BrowserThread::DeleteOnUIThread> pref_;

};

// Create and install MyMessageFilter.

MyContentBrowserClient::RenderProcessWillLaunch(RenderProcessHost* host) {

  Profile* profile = Profile::FromBrowserContext(host->GetBrowserContext());

  host->AddFilter(new MyMessageFilter(profile));

}

The problem is that my Profile object (and the PrefService that it owns) may be deleted on the UI thread before the PrefMember object is deleted on the UI thread due to the async execution of DeleteOnUIThread. This results in a heap-use-after-free when the PrefMember is deleted due to it referencing the (already deleted) PrefService.

It seems like I need to do one of the following:

A. Disconnect all PrefMembers when the PrefService is deleted, or;

B. Delay deletion of the PrefService until all PrefMembers have disconnected.

Is there a recommended way to handle this currently?

A is the right way to go. The PrefMember shouldn't have DeleteOnUIThread in the first place, because if it is correctly detached when the PrefService goes away, it can be destroyed on any thread. 

Thanks. Would it be reasonable to change PrefsMemberBase::prefs_ [1] to a WeakPtr so that the disconnect happens automatically on PrefService deletion?

[1] https://cs.chromium.org/chromium/src/components/prefs/pref_member.h?sq=package:chromium&rcl=1479975438&l=146

[Свободная Сеть]

Рады Вас приветствовать на BIGFANTV.NET - Фильмы ОНЛАЙН - это кинотеатр онлайн где Вы сможете бесплатно и в хорошем качестве. Смотреть передачи, сериалы, фильмы, ток-шоу, реалити шоу в хорошем качестве. На нашем проекте представлено больше 500 видов ток-шоу. Также в нашей библиотеке вы всегда можете наблюдать новинки русских сериалов которые обновляются только для Вас. А, в связи с колосальным развитием киноиндустрии в Украине, в нашем онлайн кинотеатре стали появляться Украинкие ток-шоу на русском и украинском языке. Благодаря нашему интерфейсу Вы легко найдете в нашем кинотеатре онлайн интересующий вас фильм или передачу онлайн. Для того чтобы сделать выбор и смотреть онлайн видео, в онлайн кинотеатре Вам поможет рейтинг по онлайн фильмам, а наш Вам совет, обратить внимание на комментарии и отзывы в соц. сетях, а также вы всегда сможете пообщаться с живыми людьми в комментариях и оставить свой отзыв о той или иной предачи онлайн. Мы всегда за розвитие сайта и наших гостей и потому очень часто вылаживаем поучительные видеоролики у себя в категориях. Вы могли заметить что мы так-же вылаживаем русские сериалы. Смотреть российские сериалы онлайн в наше время становится доступнее не только зрителям из Российской Федерации но и жителям СНГ. Помимо отечественных, наших, сериалов есть огромное количество западного кино и сериалов, мы стараемся вылаживать по мере возможности! Не давно в онлайн кинотеатре появилась категория Рыбалка и охота онлайн, где уже выложили огромное количество русских роликов. Для наших с вами детей постоянно обновляется коллекция мультфильмов и мультсериалов. А для людей постарше мы собрали колекцию Фильмов и документальных фильмов онлайн. Любителям посмеятся и порвать себе пузо мы предлагаем категорию "Я хуею" в которой вы не сможете усидеть за столом! Так-же мы предоставляем раздел советских фильмов онлайн где собрано огромное количество фильмов про войну. С каждым днем видео онлайн развивается. И сейчас чтобы смотреть кино в хорошем качестве не надо ити и платить деньги все и так доступно. Вам только надо выбрать в плеере HD 720 или HD 1080 и наслаждатся тем или иным ток шоу онлайн. Спасибо волшебному миру фильмов в 2016 году зрители смогли насладится и вернуться в парки Юрского периода, покататься по дорогам ярости, смогли совладать с невыполнимыми миссиями, вычислили виновников всех негараздов Бонда, сказали прощай экраннизированным идолам Пола Уокера, наконец-то закончили Голодные игры, посмотрели красную комнату боли, уменьшались до микроскопических размеров, отыскали повелителя миньонов, спасли весь свет от Альтрона, возвратились в минувшее и смогли исправить предстоящее, разобрались в загадке своих чувств и отправились в новый онлайн поход по далеком-далеком BIGFANTV.NET. 2016 - ый год обещал поразить всех любителей ток шоу и передач онлайн не менее острыми и запоминающимися выпусками. Мы Вас ждем! БИГФАНТВ.НЕТ всегда в теме!

http://www.bigfantv.net/news/anekdoty/1-0-209
http://www.bigfantv.net/news/mir_naiznanku/1-0-1
http://www.bigfantv.net/news/slovo_pastyrja/1-0-2
http://www.bigfantv.net/news/idealnyj_remont/1-0-3
http://www.bigfantv.net/news/mintrans/1-0-4
http://www.bigfantv.net/news/takoe_kino/1-0-5
http://www.bigfantv.net/news/shkola_remonta/1-0-6
http://www.bigfantv.net/news/vse_bude_smachno/1-0-7
http://www.bigfantv.net/news/sledstvie_veli/1-0-8
http://www.bigfantv.net/news/na_10_let_molozhe/1-0-9
http://www.bigfantv.net/news/ehkstrasensy_vedut_rassledovanie/1-0-13
http://www.bigfantv.net/news/sam_sebe_rezhisser/1-0-31
http://www.bigfantv.net/news/dva_otca_i_dva_syna/1-0-55
http://www.bigfantv.net/news/tsn/1-0-54
http://www.bigfantv.net/news/eda_kotoraja_pritvorjaetsja/1-0-81
http://www.bigfantv.net/news/golos/1-0-99
http://www.bigfantv.net/news/shuster_live/1-0-101
http://www.bigfantv.net/news/bitva_ehkstrasensov/1-0-118
http://www.bigfantv.net/news/eda_zhivaja_i_mertvaja/1-0-152
http://www.bigfantv.net/news/postskriptum/1-0-178
http://www.bigfantv.net/news/odin_den_s_mband/1-0-191
http://www.bigfantv.net/news/mesta_sily/1-0-214
http://www.bigfantv.net/news/univer_novaja_obshhaga/1-0-232
http://www.bigfantv.net/news/lotereja_schastlivoe_utro/1-0-258
http://www.bigfantv.net/news/100500_gorodov/1-0-261
http://www.bigfantv.net/news/baryshnja_krestjanka/1-0-267
http://www.bigfantv.net/news/na_noch_gljadja/1-0-266
http://www.bigfantv.net/news/marsh_brosok/1-0-265
http://www.bigfantv.net/news/jumor_jumor_jumor/1-0-245
http://www.bigfantv.net/news/oligarkh_tv/1-0-237
http://www.bigfantv.net/news/jumorina/1-0-217
http://www.bigfantv.net/news/druzja_moego_khozjaina/1-0-210
http://www.bigfantv.net/news/ne_v_dengakh_schaste/1-0-190
http://www.bigfantv.net/news/davaj_pozhenimsja/1-0-175
http://www.bigfantv.net/news/stand_up/1-0-171
http://www.bigfantv.net/news/saltykov_shhedrin/1-0-154
http://www.bigfantv.net/news/60_minut/1-0-144
http://www.bigfantv.net/news/dobrov_v_ehfire/1-0-124
http://www.bigfantv.net/news/utrennjaja_pochta_s_bratjami_ponomarenko/1-0-115
http://www.bigfantv.net/news/shuster_live/1-0-101

четверг, 24 ноября 2016 г., 5:23:06 UTC+2 пользователь Marshall Greenblatt написал:

[Bernhard Bauer]

You would still have to get that WeakPtr from somewhere, which would also require adding the capability to hand out weak pointers to PrefService, which is somewhat discouraged.

I've run into issues with BrowserMessageFilters that need to be notified about shutdown on the UI thread in the past (grr... refcounting! *shakes fist*), and I've added KeyedServiceShutdownNotifier for that. Ideally, you'd declare a dependency on PrefService, but that doesn't work because PrefService is not a KeyedService yet (for a given value of yet -- this has been an open TODO since KeyedServices have been implemented), but even without any dependencies it will get notified when the profile is destroyed, which should work for this case (as nothing will directly access the filter during profile shutdown).

Bernhard.

[Marshall Greenblatt]

Thanks, I followed the example of ShutdownNotifierFactory in extensions/browser/extension_message_filter.cc and that resolved the problem for me.

I've also filed https://bugs.chromium.org/p/chromium/issues/detail?id=669289 to fix PrintingMessageFilter.

 

Bernhard.

 

Bernhard.

 

Thanks,

Marshall

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.