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

Data loss on database modification by concurrent async execution #484

Closed
avillacis opened this issue Sep 8, 2022 · 2 comments
Closed

Data loss on database modification by concurrent async execution #484

avillacis opened this issue Sep 8, 2022 · 2 comments

Comments

@avillacis
Copy link

If several asynchronous methods, say, for several async callbacks installed for multiple libraries used in a single project, all attempt to concurrently update a single node-json-db instance, data loss will happen.

Consider the following example (nodejs v16.14.0 on Fedora 36 x86_64):

#!/usr/bin/env node
'use strict';

import { JsonDB, Config } from 'node-json-db';

let counter = 0;

const db = new JsonDB(new Config('test1-db'));

async function add_data()
{
    let k = `/record/key${counter}`;
    let record = {
        strval: `value ${counter}`,
        intval: counter
    };
    counter++;

    return await db.push(k, record, false);
}

async function main()
{
    let promiseList = [];
    for (let i = 0; i < 10; i++) {
        // NOTE: pushing the promise without awaiting for it!
        promiseList.push(add_data());
    }

    // Represent multiple async contexts, all running concurrently
    await Promise.all(promiseList);

    console.log(await db.getData('/'));
}

main()
.catch(err => console.error('Uncaught exception', err));

Expected output:

{
  record: {
    key0: { strval: 'value 0', intval: 0 },
    key1: { strval: 'value 1', intval: 1 },
    key2: { strval: 'value 2', intval: 2 },
    key3: { strval: 'value 3', intval: 3 },
    key4: { strval: 'value 4', intval: 4 },
    key5: { strval: 'value 5', intval: 5 },
    key6: { strval: 'value 6', intval: 6 },
    key7: { strval: 'value 7', intval: 7 },
    key8: { strval: 'value 8', intval: 8 },
    key9: { strval: 'value 9', intval: 9 }
  }
}

Actual output:

{ record: { key9: { strval: 'value 9', intval: 9 } } }

From a superficial examination of the code, it appears that, when attempting to fetch the previous version of the data, to merge with the new data and replace it into the internal data structure, the fetch of concurrent instance A gets suspended at the await keyword because concurrent instance B (C, D, ...) can now run. Which they do until they return control. By the time instance A gets to run again, its fetched data is obsolete. Therefore, when replacing it, data placed by other instances gets overwritten.

The use of Promise.all() is a synthetic test to expose the bug. Actual instances of this bug will happen when multiple async contexts interrupt each other when trying to update the same database, when invoked from multiple unrelated APIs in a project.

@Belphemur Belphemur added the bug label Sep 9, 2022
@Belphemur
Copy link
Owner

Belphemur commented Sep 9, 2022

Great catch and thank you for the code.
I'll make a test for it. I'll surely have to add a new dependency to have proper locking to be sure that we can't have multiple concurrent push without waiting for previous push to finish.

github-actions bot pushed a commit that referenced this issue Sep 9, 2022
# [3.0.0](v2.1.1...v3.0.0) (2022-09-09)

### Bug Fixes

* **Array:** Add support for dash in array name ([b001403](b001403)), closes [#98](#98)
* **Array:** Fix array not properly async ([7bfd98d](7bfd98d))
* **ArrayInfo:** Returns type of isValid ([d206098](d206098))
* **Array:** Support dot and number in name ([eb89a42](eb89a42)), closes [#95](#95)
* **Concurrency:** Fix issue with concurrent push from different sources ([daae2bb](daae2bb)), closes [#484](#484)
* **Config:** put proper default for ConfigWithAdapter ([dbb3b7b](dbb3b7b))
* **Convention:** Fixed quotes ([7126cad](7126cad))
* **Docs:** Be sure the doc contains Config ([c453c5d](c453c5d))
* **getIndex:** Improve documentation ([17ba435](17ba435))
* **HumanReadable:** Fix missing humanreadable ([4a2d198](4a2d198))
* **JsonAdapter:** Don't override the data property ([43898d5](43898d5))
* **Packaging:** Add type to package. ([983ea99](983ea99)), closes [#58](#58) [#57](#57)
* **README:** Update documentation ([d66d712](d66d712))
* **README:** Update documentation ([ba42a83](ba42a83)), closes [#90](#90) [#85](#85)
* **Separator:** Fix still using the slash as separator. ([c4c18b8](c4c18b8))

### Features

* **Adapter:** Add concept of adapter to read and write data ([9a31abc](9a31abc))
* **Adapter:** Let the user decide what adapter to use if they want to tweak the inner working ([975a653](975a653)), closes [#448](#448)
* add some array utils ([c85618e](c85618e))
* **Array:** Add support for nested array ([57c049f](57c049f)), closes [#422](#422) [#417](#417)
* **Async:** All the method are now async/await ([a6a4a8d](a6a4a8d)), closes [#171](#171)
* **Async:** Make the whole library async ([b99d784](b99d784)), closes [#444](#444)
* **AtomicFileAdapter:** Add support for fsync ([e4760cb](e4760cb))
* **Config:** Add Config file to setup the Database ([3915aee](3915aee))
* **Configuration:** Force giving a config object to the constructor ([8e415e6](8e415e6))
* **Date:** Add support for serializing and deserializing date type ([e62e792](e62e792)), closes [#362](#362)
* **Exists:** Add exits method ([35152a2](35152a2)), closes [#19](#19)
* **filename:** Support non json file extensions ([6be9a1d](6be9a1d))
* **Filter:** Add filtering feature ([0f7d276](0f7d276))
* **find:** Add find feature ([bd7ab4c](bd7ab4c)), closes [#17](#17)
* **FSYNC:** Optional fsync when saving the database ([8ae82ab](8ae82ab)), closes [#372](#372)
* **getIndex:** Support Numerical id ([d2e88ea](d2e88ea))
* **GetIndexValue:** Get index of a value in an array ([35d1807](35d1807)), closes [#191](#191)
* **Packaging:** Use es6 module packaging ([4487c4b](4487c4b))
* **typing:** Add basic typing to the lib for TS ([db8ab77](db8ab77))

### Performance Improvements

* **Concurrency:** Be sure that only one read or one write can be done at the same time ([1cf0038](1cf0038))
* **Config:** Easier way to import the configuration of JsonDB ([e371b71](e371b71))
* **Errors:** Export errors for easier error management in other projects ([60c90f8](60c90f8)), closes [#479](#479)

### BREAKING CHANGES

* **Async:** Every method of the library is now async and returns a promise.
* **Configuration:** We now need to receive the JsonDBConfig object in the constructor
* **Packaging:** The default export has been removed. You need to do a deconstruction import to load the library now.

import JsonDB from 'node-json-db'
becomes
import {JsonDB} from 'node-json-db'
github-actions bot pushed a commit that referenced this issue Sep 9, 2022
# [1.0.0](v0.7.3...v1.0.0) (2022-09-09)

### Bug Fixes

* **Array:** Add support for dash in array name ([b001403](b001403)), closes [#98](#98)
* **Array:** Fix array not properly async ([7bfd98d](7bfd98d))
* **ArrayInfo:** Returns type of isValid ([d206098](d206098))
* **Array:** Support dot and number in name ([eb89a42](eb89a42)), closes [#95](#95)
* **Concurrency:** Fix issue with concurrent push from different sources ([daae2bb](daae2bb)), closes [#484](#484)
* **Config:** put proper default for ConfigWithAdapter ([dbb3b7b](dbb3b7b))
* **Convention:** Fixed quotes ([7126cad](7126cad))
* **Docs:** Be sure the doc contains Config ([c453c5d](c453c5d))
* **getIndex:** Improve documentation ([17ba435](17ba435))
* **HumanReadable:** Fix missing humanreadable ([4a2d198](4a2d198))
* **JsonAdapter:** Don't override the data property ([43898d5](43898d5))
* **Packaging:** Add type to package. ([983ea99](983ea99)), closes [#58](#58) [#57](#57)
* **README:** Update documentation ([d66d712](d66d712))
* **README:** Update documentation ([ba42a83](ba42a83)), closes [#90](#90) [#85](#85)
* **Separator:** Fix still using the slash as separator. ([c4c18b8](c4c18b8))

### Features

* **Adapter:** Add concept of adapter to read and write data ([9a31abc](9a31abc))
* **Adapter:** Let the user decide what adapter to use if they want to tweak the inner working ([975a653](975a653)), closes [#448](#448)
* add some array utils ([c85618e](c85618e))
* **Array:** Add support for nested array ([57c049f](57c049f)), closes [#422](#422) [#417](#417)
* **Async:** All the method are now async/await ([a6a4a8d](a6a4a8d)), closes [#171](#171)
* **Async:** Make the whole library async ([b99d784](b99d784)), closes [#444](#444)
* **AtomicFileAdapter:** Add support for fsync ([e4760cb](e4760cb))
* **Config:** Add Config file to setup the Database ([3915aee](3915aee))
* **Configuration:** Force giving a config object to the constructor ([8e415e6](8e415e6))
* **Date:** Add support for serializing and deserializing date type ([e62e792](e62e792)), closes [#362](#362)
* **Exists:** Add exits method ([35152a2](35152a2)), closes [#19](#19)
* **filename:** Support non json file extensions ([6be9a1d](6be9a1d))
* **Filter:** Add filtering feature ([0f7d276](0f7d276))
* **find:** Add find feature ([bd7ab4c](bd7ab4c)), closes [#17](#17)
* **FSYNC:** Optional fsync when saving the database ([8ae82ab](8ae82ab)), closes [#372](#372)
* **getIndex:** Support Numerical id ([d2e88ea](d2e88ea))
* **GetIndexValue:** Get index of a value in an array ([35d1807](35d1807)), closes [#191](#191)
* **Packaging:** Use es6 module packaging ([4487c4b](4487c4b))
* **typing:** Add basic typing to the lib for TS ([db8ab77](db8ab77))

### Performance Improvements

* **Concurrency:** Be sure that only one read or one write can be done at the same time ([1cf0038](1cf0038))
* **Config:** Easier way to import the configuration of JsonDB ([e371b71](e371b71))
* **Errors:** Export errors for easier error management in other projects ([60c90f8](60c90f8)), closes [#479](#479)

### BREAKING CHANGES

* **Async:** Every method of the library is now async and returns a promise.
* **Configuration:** We now need to receive the JsonDBConfig object in the constructor
* **Packaging:** The default export has been removed. You need to do a deconstruction import to load the library now.

import JsonDB from 'node-json-db'
becomes
import {JsonDB} from 'node-json-db'
github-actions bot pushed a commit that referenced this issue Sep 9, 2022
## [2.1.2](v2.1.1...v2.1.2) (2022-09-09)

### Bug Fixes

* **Concurrency:** Fix issue with concurrent push from different sources ([daae2bb](daae2bb)), closes [#484](#484)

### Performance Improvements

* **Concurrency:** Be sure that only one read or one write can be done at the same time ([1cf0038](1cf0038))
* **Errors:** Export errors for easier error management in other projects ([60c90f8](60c90f8)), closes [#479](#479)
@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2022

🎉 This issue has been resolved in version 2.1.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Repository owner deleted a comment from github-actions bot Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants