New issue
Advanced search Search tips

Issue 863724 link

Starred by 2 users
Status: Assigned
Owner:
pwnall@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 856294



Sign in to add a comment

Document thread-safety of classes in //sql

Project Member Reported by pwnall@chromium.org, Jul 15 
The classes in //sql are generally thread-compatible (not thread-safe or thread-hostile). This should be documented using the API in //base/sequence_checker.h. Any thread-hostile class should use the API in //base/threading/thread_checker.h

This is a cleanup task. The priority is raised because without this information we can't reason about crashers like  Issue 856294 .
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 19 

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

commit 12daa3ac9311a8b8a97236e013d20adfbb0b7dc0
Author: Victor Costan <pwnall@chromium.org>
Date: Thu Jul 19 01:05:58 2018

sql: Document sql::StatementID's thread safety properties.

This CL:

1) Extracts StatementID from sql/connection.{h,cc} into its own files
   sql/statement_id.{h,cc}.
2) Adds tests specifically targeting sql::StatementID.
3) Replaces passing StatementID by const reference with passing by
   value.
4) Removes the StatementID constructor that takes a name, and updates
   the call sites to use the source file+line constructor.

Bug: 863724
Change-Id: I5bc6e160fe294189d3e9a2baa8f6be61b21c38d2
Reviewed-on: https://chromium-review.googlesource.com/1137915
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576295}
[modify] https://crrev.com/12daa3ac9311a8b8a97236e013d20adfbb0b7dc0/components/history/core/browser/url_database.cc
[modify] https://crrev.com/12daa3ac9311a8b8a97236e013d20adfbb0b7dc0/content/browser/appcache/appcache_database.cc
[modify] https://crrev.com/12daa3ac9311a8b8a97236e013d20adfbb0b7dc0/content/browser/appcache/appcache_database.h
[modify] https://crrev.com/12daa3ac9311a8b8a97236e013d20adfbb0b7dc0/sql/BUILD.gn
[modify] https://crrev.com/12daa3ac9311a8b8a97236e013d20adfbb0b7dc0/sql/connection.cc
[modify] https://crrev.com/12daa3ac9311a8b8a97236e013d20adfbb0b7dc0/sql/connection.h
[modify] https://crrev.com/12daa3ac9311a8b8a97236e013d20adfbb0b7dc0/sql/connection_unittest.cc
[add] https://crrev.com/12daa3ac9311a8b8a97236e013d20adfbb0b7dc0/sql/statement_id.cc
[add] https://crrev.com/12daa3ac9311a8b8a97236e013d20adfbb0b7dc0/sql/statement_id.h
[add] https://crrev.com/12daa3ac9311a8b8a97236e013d20adfbb0b7dc0/sql/statement_id_unittest.cc
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 23 

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

commit 3a325b81210ef49994dfcdd13137d8d5071abf41
Author: Victor Costan <pwnall@chromium.org>
Date: Mon Jul 23 22:16:18 2018

sql: Clean up some checks in sql::Statement.

This CL is extracted from https://crrev.com/c/1137851 because that CL
may uncover brokenness and get reverted. Landing these changes
separately minimizes the impact of a potential revert.

Bug: 863724
Change-Id: I9b8888faf7a736f086ca5e496f85a78e0754a0c5
Reviewed-on: https://chromium-review.googlesource.com/1145837
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577289}
[modify] https://crrev.com/3a325b81210ef49994dfcdd13137d8d5071abf41/sql/statement.cc
[modify] https://crrev.com/3a325b81210ef49994dfcdd13137d8d5071abf41/sql/statement.h
►

Sign in to add a comment