fregante/webext-options-sync

Race condition in OptionsSync constructor

karlicoss opened this issue · 1 comments

Hey, so, I tried using the library like this:

async function settExtOptions(options_object) {
    await new OptionsSync().set(options_object);
}

And I started noticing my settings being ignored. After closer inspection, seeing multiple 'Saving options' lines in logs and reading the code I realised there is race condition between _runMigrations called in OptionsSync constructor and set itself.

To be fair when I did it the way it was suggested in readme and used global variable, it stopped happening presumably due to having enough time for _runMigrations promises to run.

constructor can't be async in JS as far as I know, so fixing it properly would require extracting the migration bits in a separate method so someone could explicitly await on it to be sure; and wrapping all chrome api calls into Promises.

Happy to do it if you think it makes sense.

Indeed, there are a couple things I can do:

  • skip _runMigrations altogether when there are no migrations. Currently this check happens very late and it still reads+writes the options object
  • set a this._migration = new Promise() so it can be awaited by get/set