Use a ConcurrentHandleMap for PlacesDb connections

This commit is contained in:
Thom Chiovoloni 2019-01-17 15:42:37 -08:00 коммит произвёл Thom
Родитель c34230f63d
Коммит e78d48e27b
6 изменённых файлов: 46 добавлений и 42 удалений

1
Cargo.lock сгенерированный
Просмотреть файл

@ -1053,6 +1053,7 @@ version = "0.1.0"
dependencies = [
"android_logger 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
"ffi-support 0.1.4",
"lazy_static 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
"log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
"places 0.1.0",
"rusqlite 0.16.0 (registry+https://github.com/rust-lang/crates.io-index)",

Просмотреть файл

@ -45,17 +45,17 @@ internal interface LibPlacesFFI : Library {
db_path: String,
encryption_key: String?,
out_err: RustError.ByReference
): RawPlacesConnection?
): PlacesConnectionHandle
fun places_note_observation(
conn: RawPlacesConnection,
handle: PlacesConnectionHandle,
json_observation_data: String,
out_err: RustError.ByReference
)
/** Returns JSON string, which you need to free with places_destroy_string */
fun places_query_autocomplete(
conn: RawPlacesConnection,
handle: PlacesConnectionHandle,
search: String,
limit: Int,
out_err: RustError.ByReference
@ -65,7 +65,7 @@ internal interface LibPlacesFFI : Library {
* is provided for a slight additional amount of sanity checking. These lengths are the number
* of elements present (and not e.g. the number of bytes allocated). */
fun places_get_visited(
conn: RawPlacesConnection,
handle: PlacesConnectionHandle,
urls: StringArray,
urls_len: Int,
buffer: Pointer,
@ -74,7 +74,7 @@ internal interface LibPlacesFFI : Library {
)
fun places_get_visited_urls_in_range(
conn: RawPlacesConnection,
handle: PlacesConnectionHandle,
start: Long,
end: Long,
include_remote: Byte,
@ -82,7 +82,7 @@ internal interface LibPlacesFFI : Library {
): Pointer?
fun sync15_history_sync(
conn: RawPlacesConnection,
handle: PlacesConnectionHandle,
key_id: String,
access_token: String,
sync_key: String,
@ -94,7 +94,7 @@ internal interface LibPlacesFFI : Library {
fun places_destroy_string(s: Pointer)
/** Destroy connection created using `places_connection_new` */
fun places_connection_destroy(obj: RawPlacesConnection)
fun places_connection_destroy(handle: PlacesConnectionHandle, out_err: RustError.ByReference)
}
class RawPlacesConnection : PointerType()
internal typealias PlacesConnectionHandle = Long;

Просмотреть файл

@ -13,6 +13,7 @@ import org.json.JSONObject
import java.io.File
import java.nio.ByteBuffer
import java.nio.ByteOrder
import java.util.concurrent.atomic.AtomicLong
/**
* An implementation of a [PlacesAPI] backed by a Rust Places library.
@ -22,13 +23,13 @@ import java.nio.ByteOrder
* database. If omitted, data will be stored in plaintext.
*/
class PlacesConnection(path: String, encryption_key: String? = null) : PlacesAPI, AutoCloseable {
private var db: RawPlacesConnection?
private var handle: AtomicLong = AtomicLong(0)
init {
try {
db = rustCall { error ->
handle.set(rustCall { error ->
LibPlacesFFI.INSTANCE.places_connection_new(path, encryption_key, error)
}
})
} catch (e: InternalPanic) {
// Places Rust library does not yet support schema migrations; as a very temporary quick
@ -41,31 +42,32 @@ class PlacesConnection(path: String, encryption_key: String? = null) : PlacesAPI
File(path).delete()
db = rustCall { error ->
handle.set(rustCall { error ->
LibPlacesFFI.INSTANCE.places_connection_new(path, encryption_key, error)
}
})
}
}
@Synchronized
override fun close() {
val db = this.db
this.db = null
if (db != null) {
LibPlacesFFI.INSTANCE.places_connection_destroy(db)
val handle = this.handle.getAndSet(0L)
if (handle != 0L) {
rustCall { error ->
LibPlacesFFI.INSTANCE.places_connection_destroy(handle, error)
}
}
}
override fun noteObservation(data: VisitObservation) {
val json = data.toJSON().toString()
rustCall { error ->
LibPlacesFFI.INSTANCE.places_note_observation(this.db!!, json, error)
LibPlacesFFI.INSTANCE.places_note_observation(this.handle.get(), json, error)
}
}
override fun queryAutocomplete(query: String, limit: Int): List<SearchResult> {
val json = rustCallForString { error ->
LibPlacesFFI.INSTANCE.places_query_autocomplete(this.db!!, query, limit, error)
LibPlacesFFI.INSTANCE.places_query_autocomplete(this.handle.get(), query, limit, error)
}
return SearchResult.fromJSONArray(json)
}
@ -80,7 +82,7 @@ class PlacesConnection(path: String, encryption_key: String? = null) : PlacesAPI
rustCall { error ->
val bufferPtr = Native.getDirectBufferPointer(byteBuffer)
LibPlacesFFI.INSTANCE.places_get_visited(
this.db!!,
this.handle.get(),
urlStrings, urls.size,
bufferPtr, urls.size,
error
@ -102,7 +104,7 @@ class PlacesConnection(path: String, encryption_key: String? = null) : PlacesAPI
val urlsJson = rustCallForString { error ->
val incRemoteArg: Byte = if (includeRemote) { 1 } else { 0 }
LibPlacesFFI.INSTANCE.places_get_visited_urls_in_range(
this.db!!, start, end, incRemoteArg, error)
this.handle.get(), start, end, incRemoteArg, error)
}
val arr = JSONArray(urlsJson)
val result = mutableListOf<String>();
@ -115,7 +117,7 @@ class PlacesConnection(path: String, encryption_key: String? = null) : PlacesAPI
override fun sync(syncInfo: SyncAuthInfo) {
rustCall { error ->
LibPlacesFFI.INSTANCE.sync15_history_sync(
this.db!!,
this.handle.get(),
syncInfo.kid,
syncInfo.fxaAccessToken,
syncInfo.syncKey,

Просмотреть файл

@ -13,6 +13,7 @@ serde_json = "1.0.28"
log = "0.4"
url = "1.7.1"
ffi-support = { path = "../../support/ffi" }
lazy_static = "1.2.0"
[dependencies.rusqlite]
version = "0.16.0"

Просмотреть файл

@ -3,8 +3,8 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
use ffi_support::{
call_with_result, define_box_destructor, define_string_destructor, rust_str_from_c,
rust_string_from_c, ExternError,
define_handle_map_deleter, define_string_destructor, rust_str_from_c, rust_string_from_c,
ConcurrentHandleMap, ExternError,
};
use places::history_sync::store::HistoryStore;
use places::{storage, PlacesDb};
@ -33,6 +33,10 @@ fn logging_init() {
// XXX I'm completely punting on error handling until we have time to refactor. I'd rather not
// add more ffi error copypasta in the meantime.
lazy_static::lazy_static! {
static ref CONNECTIONS: ConcurrentHandleMap<PlacesDb> = ConcurrentHandleMap::new();
}
/// Instantiate a places connection. Returned connection must be freed with
/// `places_connection_destroy`. Returns null and logs on errors (for now).
#[no_mangle]
@ -40,10 +44,10 @@ pub unsafe extern "C" fn places_connection_new(
db_path: *const c_char,
encryption_key: *const c_char,
error: &mut ExternError,
) -> *mut PlacesDb {
) -> u64 {
log::debug!("places_connection_new");
logging_init();
call_with_result(error, || {
CONNECTIONS.insert_with_result(error, || {
let path = ffi_support::rust_string_from_c(db_path);
let key = ffi_support::opt_rust_string_from_c(encryption_key);
PlacesDb::open(path, key.as_ref().map(|v| v.as_str()))
@ -54,12 +58,12 @@ pub unsafe extern "C" fn places_connection_new(
/// Errors are logged.
#[no_mangle]
pub unsafe extern "C" fn places_note_observation(
conn: &mut PlacesDb,
handle: u64,
json_observation: *const c_char,
error: &mut ExternError,
) {
log::debug!("places_note_observation");
call_with_result(error, || {
CONNECTIONS.call_with_result_mut(error, handle, |conn| {
let json = ffi_support::rust_str_from_c(json_observation);
let visit: places::VisitObservation = serde_json::from_str(&json)?;
places::api::apply_observation(conn, visit)
@ -70,13 +74,13 @@ pub unsafe extern "C" fn places_note_observation(
/// using `places_destroy_string`. Returns null and logs on errors (for now).
#[no_mangle]
pub unsafe extern "C" fn places_query_autocomplete(
conn: &PlacesDb,
handle: u64,
search: *const c_char,
limit: u32,
error: &mut ExternError,
) -> *mut c_char {
log::debug!("places_query_autocomplete");
call_with_result(error, || {
CONNECTIONS.call_with_result(error, handle, |conn| {
search_frecent(
conn,
SearchParams {
@ -89,7 +93,7 @@ pub unsafe extern "C" fn places_query_autocomplete(
#[no_mangle]
pub unsafe extern "C" fn places_get_visited(
conn: &PlacesDb,
handle: u64,
urls: *const *const c_char,
urls_len: i32,
byte_buffer: *mut bool,
@ -98,7 +102,7 @@ pub unsafe extern "C" fn places_get_visited(
) {
log::debug!("places_get_visited");
// This function has a dumb amount of overhead and copying...
call_with_result(error, || -> places::Result<()> {
CONNECTIONS.call_with_result(error, handle, |conn| -> places::Result<()> {
assert!(
urls_len >= 0,
"Negative array length provided to places_get_visited {}",
@ -122,14 +126,14 @@ pub unsafe extern "C" fn places_get_visited(
#[no_mangle]
pub extern "C" fn places_get_visited_urls_in_range(
conn: &PlacesDb,
handle: u64,
start: i64,
end: i64,
include_remote: u8, // JNA has issues with bools...
error: &mut ExternError,
) -> *mut c_char {
log::debug!("places_get_visited_in_range");
call_with_result(error, || -> places::Result<String> {
CONNECTIONS.call_with_result(error, handle, |conn| -> places::Result<_> {
let visited = storage::history::get_visited_urls(
conn,
// Probably should allow into()...
@ -143,7 +147,7 @@ pub extern "C" fn places_get_visited_urls_in_range(
#[no_mangle]
pub unsafe extern "C" fn sync15_history_sync(
conn: &PlacesDb,
handle: u64,
key_id: *const c_char,
access_token: *const c_char,
sync_key: *const c_char,
@ -151,7 +155,7 @@ pub unsafe extern "C" fn sync15_history_sync(
error: &mut ExternError,
) {
log::debug!("sync15_history_sync");
call_with_result(error, || -> places::Result<()> {
CONNECTIONS.call_with_result(error, handle, |conn| -> places::Result<_> {
// XXX - this is wrong - we kinda want this to be long-lived - the "Db"
// should own the store, but it's not part of the db.
let store = HistoryStore::new(conn);
@ -170,4 +174,4 @@ pub unsafe extern "C" fn sync15_history_sync(
}
define_string_destructor!(places_destroy_string);
define_box_destructor!(PlacesDb, places_connection_destroy);
define_handle_map_deleter!(CONNECTIONS, places_connection_destroy);

Просмотреть файл

@ -7,11 +7,8 @@
// This module implement the traits that make the FFI code easier to manage.
use crate::api::matcher::SearchResult;
use crate::db::PlacesDb;
use crate::error::{Error, ErrorKind};
use ffi_support::{
implement_into_ffi_by_json, implement_into_ffi_by_pointer, ErrorCode, ExternError,
};
use ffi_support::{implement_into_ffi_by_json, ErrorCode, ExternError};
pub mod error_codes {
// Note: 0 (success) and -1 (panic) are reserved by ffi_support
@ -63,5 +60,4 @@ impl From<Error> for ExternError {
}
}
implement_into_ffi_by_pointer!(PlacesDb);
implement_into_ffi_by_json!(SearchResult);