diff --git a/CHANGELOG b/CHANGELOG index 1d5078856..c831696a1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,8 @@ +Version 6.0.5 (2024-06-22) +-------------------------- +Handle exceptions raised by the Executor during initialization of the thread pool for emitter (#694) +Fix concurrent modification when creating TrackerEvent by copying the payload (#692) + Version 6.0.4 (2024-06-05) -------------------------- Send correct install referrer timestamps (#687) diff --git a/VERSION b/VERSION index 1aa5e414f..288b2cd9a 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -6.0.4 +6.0.5 diff --git a/build.gradle b/build.gradle index c350e94a8..52f3b8f83 100644 --- a/build.gradle +++ b/build.gradle @@ -22,7 +22,7 @@ plugins { subprojects { group = 'com.snowplowanalytics' - version = '6.0.4' + version = '6.0.5' repositories { google() maven { diff --git a/gradle.properties b/gradle.properties index 458100ca4..98fd398ca 100644 --- a/gradle.properties +++ b/gradle.properties @@ -31,7 +31,7 @@ systemProp.org.gradle.internal.http.socketTimeout=120000 SONATYPE_STAGING_PROFILE=comsnowplowanalytics GROUP=com.snowplowanalytics POM_ARTIFACT_ID=snowplow-android-tracker -VERSION_NAME=6.0.4 +VERSION_NAME=6.0.5 POM_NAME=snowplow-android-tracker POM_PACKAGING=aar diff --git a/snowplow-tracker/src/androidTest/java/com/snowplowanalytics/snowplow/internal/emitter/storage/EventStoreTest.kt b/snowplow-tracker/src/androidTest/java/com/snowplowanalytics/snowplow/internal/emitter/storage/EventStoreTest.kt index 672f61f6e..88cb3be2a 100755 --- a/snowplow-tracker/src/androidTest/java/com/snowplowanalytics/snowplow/internal/emitter/storage/EventStoreTest.kt +++ b/snowplow-tracker/src/androidTest/java/com/snowplowanalytics/snowplow/internal/emitter/storage/EventStoreTest.kt @@ -122,7 +122,7 @@ class EventStoreTest { @Throws(InterruptedException::class) fun testInsertPayload() { val eventStore = eventStore() - val id = eventStore.insertEvent(payload()) + val id = eventStore.insertEvent(payload())!! val lastRowId = eventStore.lastInsertedRowId val event = eventStore.getEvent(id) Assert.assertEquals(id, lastRowId) @@ -153,7 +153,7 @@ class EventStoreTest { @Throws(InterruptedException::class) fun testRemoveIndividualEvent() { val eventStore = eventStore() - val id = eventStore.insertEvent(payload()) + val id = eventStore.insertEvent(payload())!! var res = eventStore.removeEvent(id) Assert.assertEquals(0, eventStore.size()) Assert.assertTrue(res) @@ -169,9 +169,9 @@ class EventStoreTest { fun testRemoveRangeOfEvents() { val eventStore = eventStore() val idList: MutableList = ArrayList() - idList.add(eventStore.insertEvent(payload())) - idList.add(eventStore.insertEvent(payload())) - idList.add(eventStore.insertEvent(payload())) + idList.add(eventStore.insertEvent(payload())!!) + idList.add(eventStore.insertEvent(payload())!!) + idList.add(eventStore.insertEvent(payload())!!) Assert.assertEquals(3, idList.size.toLong()) Assert.assertEquals(3, eventStore.size()) var res = eventStore.removeEvents(idList) diff --git a/snowplow-tracker/src/main/java/com/snowplowanalytics/core/emitter/Executor.kt b/snowplow-tracker/src/main/java/com/snowplowanalytics/core/emitter/Executor.kt index 41356b8f8..f58527101 100755 --- a/snowplow-tracker/src/main/java/com/snowplowanalytics/core/emitter/Executor.kt +++ b/snowplow-tracker/src/main/java/com/snowplowanalytics/core/emitter/Executor.kt @@ -14,10 +14,7 @@ package com.snowplowanalytics.core.emitter import androidx.annotation.RestrictTo import com.snowplowanalytics.core.tracker.Logger -import java.util.concurrent.Callable -import java.util.concurrent.ExecutorService -import java.util.concurrent.Executors -import java.util.concurrent.Future +import java.util.concurrent.* /** * Static Class which holds the logic for controlling @@ -51,11 +48,15 @@ object Executor { */ @Synchronized @JvmStatic - private fun getExecutor(): ExecutorService { + private fun getExecutor(): ExecutorService? { if (executor == null) { - executor = Executors.newScheduledThreadPool(threadCount) + try { + executor = Executors.newScheduledThreadPool(threadCount) + } catch (e: Exception) { + Logger.e("Executor", e.message ?: "Failed to create thread pool") + } } - return executor!! + return executor } /** @@ -105,7 +106,7 @@ object Executor { fun execute(runnable: Runnable?, exceptionHandler: ExceptionHandler?) { val executor = getExecutor() try { - executor.execute { + executor?.execute { try { runnable?.run() } catch (t: Throwable) { @@ -125,8 +126,13 @@ object Executor { * @return the future object to be queried */ @JvmStatic - fun futureCallable(callable: Callable<*>): Future<*> { - return getExecutor().submit(callable) + fun futureCallable(callable: Callable<*>): Future<*>? { + return try { + getExecutor()?.submit(callable) + } catch (e: Exception) { + Logger.e("Executor", e.message ?: "Failed to submit task") + null + } } /** @@ -136,7 +142,11 @@ object Executor { @JvmStatic fun shutdown(): ExecutorService? { if (executor != null) { - executor!!.shutdown() + try { + executor?.shutdown() + } catch (e: Exception) { + Logger.e("Executor", e.message ?: "Failed to shutdown") + } val es = executor executor = null return es diff --git a/snowplow-tracker/src/main/java/com/snowplowanalytics/core/emitter/storage/SQLiteEventStore.kt b/snowplow-tracker/src/main/java/com/snowplowanalytics/core/emitter/storage/SQLiteEventStore.kt index 75ae8f8aa..da26f30ce 100755 --- a/snowplow-tracker/src/main/java/com/snowplowanalytics/core/emitter/storage/SQLiteEventStore.kt +++ b/snowplow-tracker/src/main/java/com/snowplowanalytics/core/emitter/storage/SQLiteEventStore.kt @@ -41,7 +41,7 @@ import kotlin.time.Duration class SQLiteEventStore(context: Context, private val namespace: String) : EventStore { private val payloadWaitingList: MutableList = ArrayList() private var database: SQLiteDatabase? = null - private lateinit var dbHelper: EventStoreHelper + private var dbHelper: EventStoreHelper? = null private val allColumns = arrayOf( EventStoreHelper.COLUMN_ID, EventStoreHelper.COLUMN_EVENT_DATA, @@ -62,7 +62,7 @@ class SQLiteEventStore(context: Context, private val namespace: String) : EventS * @return a boolean for database status */ val databaseOpen: Boolean - get() = database != null && database!!.isOpen + get() = database != null && database?.isOpen ?: false /** * Creates a new Event Store @@ -91,7 +91,7 @@ class SQLiteEventStore(context: Context, private val namespace: String) : EventS */ fun open() { if (!databaseOpen) { - database = dbHelper.writableDatabase + database = dbHelper?.writableDatabase database?.enableWriteAheadLogging() } } @@ -100,7 +100,7 @@ class SQLiteEventStore(context: Context, private val namespace: String) : EventS * Closes the database */ fun close() { - dbHelper.close() + dbHelper?.close() EventStoreHelper.removeInstance(namespace) } @@ -112,25 +112,27 @@ class SQLiteEventStore(context: Context, private val namespace: String) : EventS * @return a boolean stating if the insert * was a success or not */ - fun insertEvent(payload: Payload): Long { + fun insertEvent(payload: Payload): Long? { if (databaseOpen) { + val database = database ?: return null val bytes = Util.serialize(Util.objectMapToString(payload.map)) val values = ContentValues(2) values.put(EventStoreHelper.COLUMN_EVENT_DATA, bytes) lastInsertedRowId = - database!!.insert(EventStoreHelper.TABLE_EVENTS, null, values) + database.insert(EventStoreHelper.TABLE_EVENTS, null, values) + Logger.d(TAG, "Added event to database: %s", lastInsertedRowId) + return lastInsertedRowId } - Logger.d(TAG, "Added event to database: %s", lastInsertedRowId) - return lastInsertedRowId + return null } override fun removeEvent(id: Long): Boolean { var retval = -1 if (databaseOpen) { - retval = database!!.delete( + retval = database?.delete( EventStoreHelper.TABLE_EVENTS, EventStoreHelper.COLUMN_ID + "=" + id, null - ) + ) ?: retval } Logger.d(TAG, "Removed event from database: %s", "" + id) return retval == 1 @@ -142,10 +144,10 @@ class SQLiteEventStore(context: Context, private val namespace: String) : EventS } var retval = -1 if (databaseOpen) { - retval = database!!.delete( + retval = database?.delete( EventStoreHelper.TABLE_EVENTS, EventStoreHelper.COLUMN_ID + " in (" + Util.joinLongList(ids) + ")", null - ) + ) ?: retval } Logger.d(TAG, "Removed events from database: %s", retval) return retval == ids.size @@ -155,7 +157,7 @@ class SQLiteEventStore(context: Context, private val namespace: String) : EventS var retval = 0 Logger.d(TAG, "Removing all events from database.") if (databaseOpen) { - retval = database!!.delete(EventStoreHelper.TABLE_EVENTS, null, null) + retval = database?.delete(EventStoreHelper.TABLE_EVENTS, null, null) ?: retval } else { Logger.e(TAG, "Database is not open.") } @@ -196,9 +198,10 @@ class SQLiteEventStore(context: Context, private val namespace: String) : EventS private fun queryDatabase(query: String?, orderBy: String?): List> { val res: MutableList> = ArrayList() if (databaseOpen) { + val database = database ?: return res var cursor: Cursor? = null try { - cursor = database!!.query( + cursor = database.query( EventStoreHelper.TABLE_EVENTS, allColumns, query, diff --git a/snowplow-tracker/src/main/java/com/snowplowanalytics/core/tracker/TrackerEvent.kt b/snowplow-tracker/src/main/java/com/snowplowanalytics/core/tracker/TrackerEvent.kt index f5e5e7185..3967b9ab9 100644 --- a/snowplow-tracker/src/main/java/com/snowplowanalytics/core/tracker/TrackerEvent.kt +++ b/snowplow-tracker/src/main/java/com/snowplowanalytics/core/tracker/TrackerEvent.kt @@ -43,8 +43,12 @@ class TrackerEvent @JvmOverloads constructor(event: Event, state: TrackerStateSn init { entities = event.entities.toMutableList() trueTimestamp = event.trueTimestamp - payload = HashMap(event.dataPayload) - + // NOTE: this code is a workaround since the types of the `Event.dataPayload` and `TrackerEvent.payload` don't match + // `Event` allows the values to be optional, while `TrackerEvent` does not. + // We should fix this in the next major version to unify the types. + // The `toMap()` is called in order to make a copy before calling HashMap as that can lead to concurrency issues, see #692 + payload = HashMap(event.dataPayload.toMap()) + if (state != null) { this.state = state } else { diff --git a/snowplow-tracker/src/main/java/com/snowplowanalytics/snowplow/network/OkHttpNetworkConnection.kt b/snowplow-tracker/src/main/java/com/snowplowanalytics/snowplow/network/OkHttpNetworkConnection.kt index 4092cc254..4482da2e9 100644 --- a/snowplow-tracker/src/main/java/com/snowplowanalytics/snowplow/network/OkHttpNetworkConnection.kt +++ b/snowplow-tracker/src/main/java/com/snowplowanalytics/snowplow/network/OkHttpNetworkConnection.kt @@ -248,7 +248,7 @@ class OkHttpNetworkConnection private constructor(builder: OkHttpNetworkConnecti request, userAgent ) else buildPostRequest(request, userAgent) - futures.add(Executor.futureCallable(getRequestCallable(okHttpRequest))) + Executor.futureCallable(getRequestCallable(okHttpRequest))?.let { futures.add(it) } } Logger.d(TAG, "Request Futures: %s", futures.size)