objectbox-java icon indicating copy to clipboard operation
objectbox-java copied to clipboard

Query returns same result even when different parameter value given

Open AngryGami opened this issue 3 years ago • 8 comments

Describe the bug Despite explicit setting different parameter value subsequent query executions return same result sometimes (if made quickly enough, or maybe two different parameter values have same hash?)

Basic info (please complete the following information):

  • ObjectBox version: 3.1.2
  • Reproducibility: occasionally without visible pattern
  • Device: emulator, Sony Xperia XII
  • OS: Android 10

To Reproduce See code section

Expected behavior For different parameters values results must be different.

Code

@Entity
class KAEntry(
    @Unique val key: String,
    val value: String,
    @Id var id: Long = 0
)
...

val boxStore: BoxStore = MyObjectBox.builder().androidContext(context).build()
val kAEntryBox = boxStore.boxFor(KAEntry::class.java)
val byKeyQuery = kAEntryBox.query().equal(KAEntry_.key, "", CASE_SENSITIVE).parameterAlias("key").build()

kAEntryBox.put(KAEntry("currentUser.lastUnlockTimestamp","123123"))
kAEntryBox.put(KAEntry("currentUser.navigationStack","some other value"))

val result1: KAEntry? = byKeyQuery.setParameter("key", "currentUser.lastUnlockTimestamp").findUnique()
val result2: KAEntry? = byKeyQuery.setParameter("key", "currentUser.navigationStack").findUnique()

if (result1?.key == result2?.key) { 
// this shouldn't be possible since I'm querying by different keys.
     error("Query returned same result twice")
} else {
    // fine this time
}

Additional context I assumed that this is issue with reuse of byKeyQuery object and instead of reusing it I create new query every time. I'm not 100% sure yet if this helped since this issue is very sporadic, but I'll know if it reappears.

AngryGami avatar Apr 20 '22 14:04 AngryGami

Really weird. Will it become reproducible if you put the relevant code in a loop?

cc @ivahnenkoAnna @greenrobot-team

greenrobot avatar Apr 20 '22 20:04 greenrobot

I think you have to put this code in a loop (query part) to reproduce issue. In my project this happens very rarely but usually when two requests are executed in rapid succession. Here are log lines produced by my application when this happens:

04-20 13:19:57.916 D/PropsRepository(19599): setPropValue existing.key = currentUser.navigationStack [23] for currentUser.lastUnlockTimestamp
04-20 13:19:57.916 D/PropsRepository(19599): setPropValue existing.key = currentUser.navigationStack [23] for currentUser.navigationStack

And here how code that logs this looks like:

{ it:Map.Entry<String, Any> ->
val existing: KAEntry? = byKeyQuery.setParameter("key", it.key).findUnique()
logd { "setPropValue existing.key = ${existing?.key} [${existing?.id}] for ${it.key}" }
// do some other stuff 
}

You can see that it.key had value currentUser.lastUnlockTimestamp in first query and currentUser.navigationStack in second one and in both cases existing.key was currentUser.navigationStack. This happened very quickly (milliseconds haven't changed even) and in same thread (19599), so this unlikely caused by concurrent access.

AngryGami avatar Apr 20 '22 22:04 AngryGami

Thanks for the additional details. However, I failed to reproduce this. I tested using a local unit test on Windows and an instrumentation test on an Android 12 emulator.

Used test code like:

@Entity
public class Customer {
    @Id long id;
    @Unique String name;
}

customerBox.put(new Customer(0, "first"));
customerBox.put(new Customer(0, "second"));

try (Query<Customer> query = customerBox.query()
        .equal(Customer_.name, "", QueryBuilder.StringOrder.CASE_SENSITIVE)
        .parameterAlias("key")
        .build()) {
    for (int i = 0; i < 10000; i++) {
        query.setParameter("key", "first");
        assertEquals("first", getUniqueNotNull(query).getName());

        query.setParameter("key", "second");
        assertEquals("second", getUniqueNotNull(query).getName());
    }
}

If you are happy to spend the time, could you provide a small example project that reproduces this issue?

greenrobot-team avatar Apr 26 '22 13:04 greenrobot-team

What getUniqueNotNull do? I can guess from name :) but would prefer not to.

AngryGami avatar Apr 26 '22 13:04 AngryGami

I think that this is a race condition during call to setParameter on byKeyQuery object (i.e. one thread set parameter and another changed it underneath before findUnique happen or used its value). I can consistently reproduce this scenario in my test project and this also means that this is not ObjectBox problem but rather me using byKeyQuery in non multi-thread safe way. Though I still think this api needs some love so e.g. setParameter returns new object every time and therefore can be re-used from multiple threads.

AngryGami avatar Apr 26 '22 16:04 AngryGami

@AngryGami Agreed.

  1. I think we should mention this here and how to do it safely (e.g. synchronize on the query object, or build per-thread): https://docs.objectbox.io/queries#reusing-queries-and-parameters (e.g. info box)
  2. We have Query cloning internally, and I think we should make available to Java too.

greenrobot avatar Apr 27 '22 11:04 greenrobot

While this is the way it is (i.e. non-safe to call setParameter from different threads) - how big of impact it will be to re-create query every time (workaround I've mentioned in very first message)?

AngryGami avatar Apr 27 '22 14:04 AngryGami

how big of impact it will be to re-create query every time

If it's not a hotspot, it is usually fine.

greenrobot avatar Apr 27 '22 20:04 greenrobot

https://docs.objectbox.io/queries#reusing-queries-and-parameters now has a note about re-using queries across multiple threads.

Also with the new 3.5.0 release, it is possible to create a copy of a Query using copy() to use it in a different thread. It also includes QueryThreadLocal which mirrors the behavior of ThreadLocal, but gets a Query copy for each thread.

Let us know if there are issues!

greenrobot-team avatar Dec 05 '22 13:12 greenrobot-team

https://docs.objectbox.io/queries#reusing-queries-and-parameters now has a note about re-using queries across multiple threads.

Also with the new 3.5.0 release, it is possible to create a copy of a Query using copy() to use it in a different thread. It also includes QueryThreadLocal which mirrors the behavior of ThreadLocal, but gets a Query copy for each thread.

Let us know if there are issues!

Great. But we need it in GO SDK.

chzhongsending avatar Aug 01 '23 03:08 chzhongsending