Status: Unconfirmed
Owner: ----
Labels: OS-Linux Area-Misc Size-Medium Type-Bug

New issue 18488 by timurrrr: Possible data race on HistoryAddPageArgs
http://code.google.com/p/chromium/issues/detail?id=18488

Chrome Version       : r22179
OS + version : Linux, possibly Mac as well

Hi,
I've been running ui_tests under ThreadSanitizer and got the following race
report:

==21334== INFO: T0 is program's main thread
==21334== INFO: T7 has been created by T0 at this point: {{{
==21334==     #0  clone /lib32/libc-2.7.so
==21334==     #1  pthread_create@@GLIBC_2.1 /lib32/libpthread-2.7.so
==21334==     #2  pthread_cre...@*
/home/timurrrr/valgrind-patches/valgrind-10695/tsan/ts_valgrind_intercepts.c:556
==21334==     #3  (anonymous namespace)::CreateThread(unsigned int, bool,
PlatformThread::Delegate*, unsigned long*) base/platform_thread_posix.cc:93
==21334==     #4  PlatformThread::Create(unsigned int,
PlatformThread::Delegate*, unsigned long*) base/platform_thread_posix.cc:105
==21334==     #5  base::Thread::StartWithOptions(base::Thread::Options
const&) base/thread.cc:84
==21334==     #6  base::Thread::Start() base/thread.cc:73
==21334==     #7  HistoryService::Init(FilePath const&, BookmarkService*)
chrome/browser/history/history.cc:128
==21334==     #8
ProfileImpl::GetHistoryService(Profile::ServiceAccessType)
chrome/browser/profile.cc:862
==21334==     #9  VisitedLinkMaster::RebuildTableFromHistory()
chrome/browser/visitedlink_master.cc:868
==21334==     #10 VisitedLinkMaster::InitFromScratch(bool)
chrome/browser/visitedlink_master.cc:622
==21334==     #11 VisitedLinkMaster::Init()
chrome/browser/visitedlink_master.cc:265
==21334==     #12 ProfileImpl::GetVisitedLinkMaster()
chrome/browser/profile.cc:721
==21334==     #13 BrowserRenderProcessHost::InitVisitedLinks()
chrome/browser/renderer_host/browser_render_process_host.cc:576
==21334==     #14 BrowserRenderProcessHost::Init()
chrome/browser/renderer_host/browser_render_process_host.cc:492
==21334==     #15 RenderViewHost::CreateRenderView()
chrome/browser/renderer_host/render_view_host.cc:174
==21334== WARNING: Possible data race during read of size 4 at 0xB79C8F0:  
{{{
==21334==    T0 (locks held: {}):
==21334==     #0  std::vector<GURL, std::allocator<GURL> >::~vector()
/usr/include/c++/4.2/bits/stl_vector.h:268
==21334==     #1  history::HistoryAddPageArgs::~HistoryAddPageArgs()
chrome/browser/history/history_marshaling.h:21
==21334==     #2  base::RefCounted<history::HistoryAddPageArgs>::Release()
base/ref_counted.h:80
==21334==     #3
scoped_refptr<history::HistoryAddPageArgs>::~scoped_refptr()
base/ref_counted.h:196
==21334==     #4  HistoryService::AddPage(GURL const&, base::Time, void
const*, int, GURL const&, unsigned int, std::vector<GURL,
std::allocator<GURL> > const&, bool) chro»
==21334==     #5  HistoryService::AddPage(GURL const&, void const*, int,
GURL const&, unsigned int, std::vector<GURL, std::allocator<GURL> > const&,
bool) chrome/browser/h»
==21334==     #6  TabContents::UpdateHistoryForNavigation(GURL const&,
NavigationController::LoadCommittedDetails const&,
ViewHostMsg_FrameNavigate_Params const&) chrome/b»
==21334==     #7  TabContents::DidNavigate(RenderViewHost*,
ViewHostMsg_FrameNavigate_Params const&)
chrome/browser/tab_contents/tab_contents.cc:1848
==21334==     #8  RenderViewHost::OnMsgNavigate(IPC::Message const&)
chrome/browser/renderer_host/render_view_host.cc:944
==21334==     #9  RenderViewHost::OnMessageReceived(IPC::Message const&)
chrome/browser/renderer_host/render_view_host.cc:723
==21334==   Concurrent write(s) happened at (OR AFTER) these points:
==21334==    T7 (locks held: {}):
==21334==     #0  std::vector<GURL, std::allocator<GURL>
> ::erase(__gnu_cxx::__normal_iterator<GURL*, std::vector<GURL,
std::allocator<GURL> > >) /usr/include/c++/4.2/bits»
==21334==     #1
history::HistoryBackend::AddPage(scoped_refptr<history::HistoryAddPageArgs>)  
chrome/browser/history/history_backend.cc:404
==21334==     #2  void DispatchToMethod<history::HistoryBackend, void
(history::HistoryBackend::*)(scoped_refptr<history::HistoryAddPageArgs>),
scoped_refptr<history::Hist»
==21334==     #3  RunnableMethod<history::HistoryBackend, void
(history::HistoryBackend::*)(scoped_refptr<history::HistoryAddPageArgs>),
Tuple1<scoped_refptr<history::Hist»
==21334==     #4  MessageLoop::RunTask(Task*) base/message_loop.cc:316
==21334==     #5
MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&)
base/message_loop.cc:324
==21334==     #6  MessageLoop::DoWork() base/message_loop.cc:435
==21334==     #7
base::MessagePumpDefault::Run(base::MessagePump::Delegate*)
base/message_pump_default.cc:23
==21334==     #8  MessageLoop::RunInternal() base/message_loop.cc:199
==21334==     #9  MessageLoop::RunHandler() base/message_loop.cc:181
==21334== }}}

Please note that HistoryAddPageArgs subclasses base::RefCounted, NOT
base::RefCountedThreadSafe
(chrome/browser/history/history_marshaling.h)
  20 // Marshalling structure for AddPage.
  21 class HistoryAddPageArgs : public base::RefCounted<HistoryAddPageArgs> {
  22  public:

and scoped_ptr<HistoryAddPageArgs> are shared between multiple threads:
(chrome/browser/history/history.cc, executed in Thread #0)
  295
  296   scoped_refptr<history::HistoryAddPageArgs> request(
  297       new history::HistoryAddPageArgs(url, time, id_scope, page_id,
  298                                       referrer, redirects, transition,
  299                                       did_replace_entry));
  300   ScheduleAndForget(PRIORITY_NORMAL, &HistoryBackend::AddPage,  
request);
  301 }

(chrome/browser/history/history_backend.cc, executed in Thread #7)
  321
  322 void HistoryBackend::AddPage(scoped_refptr<HistoryAddPageArgs>  
request) {
  323   DLOG(INFO) << "Adding page " << request->url.possibly_invalid_spec();
  324

I believe the fix is simple: just replace base::RefCounted with
base::RefCountedThreadSafe

Thank you,
Timur Iskhodzhanov

--
You received this message because you are listed in the owner
or CC fields of this issue, or because you starred this issue.
You may adjust your issue notification preferences at:
http://code.google.com/hosting/settings

--~--~---------~--~----~------------~-------~--~----~
Automated mail from issue updates at http://crbug.com/
Subscription options: http://groups.google.com/group/chromium-bugs
-~----------~----~----~----~------~----~------~--~---

Reply via email to