-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Fix the race condition when closing one of multiple databases. #8515
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
Fix the race condition when closing one of multiple databases. #8515
Conversation
Only need disconnect the mrview with right database name, in case multiple databases running.
|
Thanks @cetsupport and @pubkey for reporting this. After upgrading from PouchDB 7.2.2 to 7.3.0, I noticed the same problem in several projects where I use several databases simultaneously. Two inappropriate behaviors occur consistently:
Both behaviors are fixed when manually applying the patch from this MR, so I'd be really glad to see it merged and prioritized for the next patch release 🙏 |
|
I just saw that version 7.3.1 was released but did not include this fix. Could it be prioritized for the next patch release or is extra work needed before? If so, how can we help? @AlbaHerrerias, as last person having published a release, would you maybe have an update about this issue? Thanks in advance. |
|
@maximerety One thing that would help get this merged is having a unit test in the PR. Could you look at adding that, than I can help get this reviewed. |
I've included both in my PR: #8513. |
|
Merged in #8513 |
Hi @garethbowen, Thanks for your answer. I'm sorry I didn't respond sooner... I tried to push the tests for this issue, then I started having too many questions making me wonder if I was on the right track. I have created an issue summarizing my concerns here: #8574 (+ #8575) If you have time to have a look someday, it would be greatly appreciated. @cetsupport / @PaulMest Thanks again for the fix. If you have time to give your opinion on #8574, I would gladly take it under advisement. |
|
Got it, will check it later today or tomorrow. BR//Jeff |
|
Sorry I leave for vacation couple of days, please let me know if I need review it or not? @maximerety |
@cetsupport Thank you for taking the time to revisit this issue. I would certainly appreciate your opinion on #8574 (+ #8575) as it has received little feedback, albeit positive, to date. |
Only need disconnect the mrview with right database name, in case multiple databases running.
Issue originally reported here: pubkey/rxdb#3813.
PR with detail reproduce procedure is here: #8513