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

openBox/openLazyBox can potentially open a Box twice #345

Closed
filipproch opened this issue Jun 13, 2020 · 10 comments
Closed

openBox/openLazyBox can potentially open a Box twice #345

filipproch opened this issue Jun 13, 2020 · 10 comments
Assignees
Labels
problem An unconfirmed bug.

Comments

@filipproch
Copy link

filipproch commented Jun 13, 2020

Steps to Reproduce
Call openBox or openLazyBox twice in succession with the same arguments. The result will be two different instances (checked by "hashCode").

This causes issues when writing to the database since notifications about changes are tied to a each instance. To obtain new data, one needs to close and open again.

To solve this, I am using https://pub.dev/packages/synchronized

But I believe this should be handled by the library, as I personally, just to understand where the issue lays, spent a few days of "WTF is happening".

Code sample

EDIT:

Here is a reproduction example: https://github.com/filipproch/hive_bug_synchronization_reproduction

Run it and you will get multiple instances of "Box" (in my case for example)

box[817852974]
box[96392022]
box[718610807]
box[383865212]
box[383865212]
box[383865212]
box[383865212]
box[383865212]
box[383865212]
box[383865212]
box[383865212]
box[383865212]

The following example is simplified, to accent the problem

Box instance1;
Hive.openBox('test')
  .then((it) { instance1 = it; });

Box instance2;
Hive.openBox('test')
  .then((it) { instance2 = it; });

// instance1.hashCode != instance2.hashCode

My solution

final lock = Lock();

Box instance1;
lock.synchronized(() async => await Hive.openBox('test'))
  .then((it) { instance1 = it; });

Box instance2;
lock.synchronized(() async => await Hive.openBox('test'))
  .then((it) { instance2 = it; });

// instance1.hashCode == instance2.hashCode

Version

  • Platform: iOS, Android
  • Flutter version: Channel stable, v1.17.3, on Mac OS X 10.15.4 19E287, locale en-US
  • Hive version: v1.4.1+1
@filipproch filipproch added the problem An unconfirmed bug. label Jun 13, 2020
@jonataslaw
Copy link

openBox is a Future, which means that the compiler will move to the next line before the result of the variable, until the end of the Future, instance1 and instance2 are two different Futures objects. When the result arrives, they will be pointed to the same reference in memory.
I think you don't need an external package for this, you just need to do this:

final instance1 = await Hive.openBox('test');
final instance2 = await Hive.openBox('test');

// instance1.hashCode == instance2.hashCode

Thus, I believe that using await there is no possibility to open the box twice.

@filipproch
Copy link
Author

There is, I am actually awaiting the Future - I mentioned this is very simplified

@filipproch
Copy link
Author

filipproch commented Jun 16, 2020

And I believe, it should be prevented on the library level, to open a box twice, since it leads to very unpredictable behavior.

@jonataslaw
Copy link

Well, I guess you didn't quite understand what I meant, so I'll try to explain it better.
Currently in Dart we have 2 types of variables, asynchronous, and synchronous (this will change, the "late" variables will be added).

When you create a variable, it is always a "new" instance.

class Foo{}

final a = Foo();
final b = Foo();

print(a==b); // false

When you declare a "Future", you are calling the factory of the Future class, and this will always return a new instance.
If you have 150 instances of Futures, they are all different, but when you call await, all instances that are called with await can receive a global instance.

import 'dart:async';

void main() {
  final a = asyncFunction();
  final b = asyncFunction2();
  print(a==b); 
  //Console: false
}

String global = 'global';

Future<String> asyncFunction() {
  return Future.value(global);
}

Future<String> asyncFunction2() {
  return Future.value(global);
}

When you declare an instance of a Future, the dart creates an instance of a Future, so that you can get the promised value with an await / then. The delivered object is not the object you are waiting for, it is just a factory, so you can get the instance, function or variable you want with an await/then.

import 'dart:async';

void main() async {
  final a = await asyncFunction();
  final b = await asyncFunction2();
  print(a==b); 
  //Console: true
}

String global = 'global';

Future<String> asyncFunction() {
  return Future.value(global);
}

Future<String> asyncFunction2() {
  return Future.value(global);
}

Despite always creating a different instance of Future, the created object will always be the same instance when called with an await/then

import 'dart:async';

void main() async {
  final a = asyncFunction();
  final b = asyncFunction2();
  print(a==b); 
  // false
  
  final c = await a;
  final d = await b;
  
  print(c ==d);
  // true
}

String global = 'global';

Future<String> asyncFunction() {
  return Future.value(global);
}

Future<String> asyncFunction2() {
  return Future.value(global);
}

So, you are not opening two boxes, you will only open the first, the second will give you the same object as the first. This is standard dart behavior, and the box will only open once on the hive, as hive checks if there is an open box.
I know that at first glance, it seems that everything is being opened twice (as in example 2), but with example 3 I believe it has become clear that this does not happen.

@filipproch
Copy link
Author

filipproch commented Jun 16, 2020

Thank you for the detailed explanation.

But it is actually opened twice, I have a working (well broken) example - where I can get two boxes to open and notifications of changes not being delivered.

EDIT:
I updated the code example for clarity, because I think you misuderstood my issue

@themisir
Copy link
Contributor

image

awaiting open(Lazy)Box es works as expected.

@filipproch
Copy link
Author

image

awaiting open(Lazy)Box es works as expected.

Awaiting works as expected, the problem is when you don't await them like so (in the synchronous matter), which can easily happen, when you call "openBox" from different parts of the app.

Then, you can easily get two different instances.

I will try to give you a test scenario which fails.

@themisir
Copy link
Contributor

themisir commented Jul 24, 2020

image
awaiting open(Lazy)Box es works as expected.

Awaiting works as expected, the problem is when you don't await them like so (in the synchronous matter), which can easily happen, when you call "openBox" from different parts of the app.

Then, you can easily get two different instances.

I will try to give you a test scenario which fails.

But you have to await openBox anyway :D that's why it's async function btw. I think this is not a issue. This issue may occur on dart internal functions too (eg: file read / write functions).

@filipproch
Copy link
Author

image
awaiting open(Lazy)Box es works as expected.

Awaiting works as expected, the problem is when you don't await them like so (in the synchronous matter), which can easily happen, when you call "openBox" from different parts of the app.
Then, you can easily get two different instances.
I will try to give you a test scenario which fails.

But you have to await openBox anyway :D that's why it's async function btw. I think this is not a issue. This issue may occur on dart internal functions too (eg: file read / write functions).

First, I am really disappointed in people replying in this thread (@themisir ) , since all you do is trying to make me the stupid one, who doesn't understand Dart/Flutter. I would appreciate if you tried and instead were open-minded and thought about the problem, instead of fast "bullshit" replies. It's just useless for the open-source community and discourages people like me to contribute and help.

I understand my explanations are not perfect, but, well it's not easy to explain complex problems in a few sentences/short samples of code.

With that out of way -

No, I think you don't understand. I do "await" the opening of box, that's not the problem. I don't await two of them synchronously, in one file one after another.

The problem is that having two separate "classes", which both open the same box inside, using them from separate parts of the app. If by chance, you call the "openBox" function at the same time - awaiting results - Dart doesn't guarantee that they will happen one after another and this library doesn't handle this either.

Here is a reproduction example: https://github.com/filipproch/hive_bug_synchronization_reproduction

Run it and you will get multiple instances of "Box" (in my case for example)

box[817852974]
box[96392022]
box[718610807]
box[383865212]
box[383865212]
box[383865212]
box[383865212]
box[383865212]
box[383865212]
box[383865212]
box[383865212]
box[383865212]

@themisir
Copy link
Contributor

themisir commented Jul 25, 2020

Thanks, I understand. Maybe I could cache openBox requests too. I will think about how to fix that issue. Sorry for misunderstanding ):

themisir added a commit that referenced this issue Jul 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
problem An unconfirmed bug.
Projects
None yet
Development

No branches or pull requests

4 participants