Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query returns same result even when different parameter value given #1071

Closed
AngryGami opened this issue Apr 20, 2022 · 10 comments
Closed

Query returns same result even when different parameter value given #1071

AngryGami opened this issue Apr 20, 2022 · 10 comments
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Milestone

Comments

@AngryGami
Copy link

AngryGami commented Apr 20, 2022

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 AngryGami added the bug Something isn't working label Apr 20, 2022
@greenrobot
Copy link
Member

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

cc @ivahnenkoAnna @greenrobot-team

@AngryGami
Copy link
Author

AngryGami commented Apr 20, 2022

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.

@greenrobot-team
Copy link
Member

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 greenrobot-team added the more info required Further information is requested label Apr 26, 2022
@AngryGami
Copy link
Author

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

@AngryGami
Copy link
Author

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.

@greenrobot
Copy link
Member

@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.

@AngryGami
Copy link
Author

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)?

@greenrobot
Copy link
Member

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

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

@greenrobot greenrobot removed bug Something isn't working more info required Further information is requested labels Apr 27, 2022
@greenrobot-team greenrobot-team added enhancement New feature or request documentation Improvements or additions to documentation labels May 2, 2022
@greenrobot-team greenrobot-team added this to the 3.4.1 milestone Nov 29, 2022
@greenrobot-team
Copy link
Member

greenrobot-team commented Dec 5, 2022

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!

@chzhongsending
Copy link

chzhongsending commented Aug 1, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants