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

Remove localstorage limit #13465

Open
ccorcos opened this issue Jun 27, 2018 · 16 comments

Comments

Projects
None yet
7 participants
@ccorcos
Copy link

commented Jun 27, 2018

  • Electron Version: 2.0.2
  • Operating System (Platform and Version): MacOS
  • Last known working Electron version: 1.8.1 (never had this issue before in an older version of the app)

Sorry for the extremely vague bug report, but I found a serious bug in our app where localStorage seems to have stopped working without throwing any errors causing using users to lose data.

localStorage.setItem("x", "10")
> undefined
localStorage.getItem("x")
> undefined

Upon deleting the Application Support files, everything started working again. Is it possible that we are not raising an error in some cases when localStorage doesn't work? Perhaps the data has been corrupted or something?

I apologize that I don't have reproducible steps, but I think its worth documenting in case others have a similar issue.

@ccorcos

This comment has been minimized.

Copy link
Author

commented Jun 28, 2018

Alright, I have a repro!

With the Electron 2.0.2 application, from the dev tools:

localStorage.length
> 635
for (let i = 0; i < 10000; i++) { localStorage.setItem(i.toString(), Array(1000).fill(1).toString()) }
> undefined
localStorage.length
> 5630
for (let i = 10000; i < 20000; i++) { localStorage.setItem(i.toString(), Array(1000).fill(1).toString()) }
> undefined
localStorage.length
> 5630

I when I try to add more, it doesn't work and there are no errors!

In Electron 1.8.1 application, from the dev tools:

localStorage.length
> 359
for (let i = 0; i < 10000; i++) { localStorage.setItem(i.toString(), Array(1000).fill(1).toString()) }
> undefined
localStorage.length
> 10359
for (let i = 10000; i < 20000; i++) { localStorage.setItem(i.toString(), Array(1000).fill(1).toString()) }
> undefined
localStorage.length
> 20008
for (let i = 0; i < 800000; i++) { localStorage.setItem(i.toString(), Array(1000).fill(1).toString()) }
> undefined
localStorage.length
> 800008

Now, I'm not sure why we appear to have lost some entries -- its possible that the application itself removed some stuff -- but for the most part, it worked. It took about 2 minutes to add the 800000 entries, so its definitely working.

Sounds like a bug to me!

@bpasero

This comment has been minimized.

Copy link
Contributor

commented Jun 28, 2018

@ccorcos does it reproduce if you run this early before app.on("ready"):

app.commandLine.appendSwitch('disable-mojo-local-storage');

@ccorcos

This comment has been minimized.

Copy link
Author

commented Jun 28, 2018

localStorage.length
> 155
for (let i = 0; i < 10000; i++) { localStorage.setItem(i.toString(), Array(1000).fill(1).toString()) }
> undefined
localStorage.length
> 10155
for (let i = 10000; i < 20000; i++) { localStorage.setItem(i.toString(), Array(1000).fill(1).toString()) }
> undefined
localStorage.length
> 20155
for (let i = 0; i < 800000; i++) { localStorage.setItem(i.toString(), Array(1000).fill(1).toString()) }
> undefined
localStorage.length
> 800009

Seems to be the culprit!

@bpasero

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2018

This looks like an issue with localStorage using levelDB then (which can be turned off using that command line switch). @ccorcos could you please try to reproduce your original issue with Electron 3.0 beta?

@ccorcos

This comment has been minimized.

Copy link
Author

commented Jul 4, 2018

Hey @bpasero would you mind trying? 3.0.0-beta.1 keeps crashing :/

@bpasero

This comment has been minimized.

Copy link
Contributor

commented Jul 7, 2018

@ccorcos I think this still reproduces in Electron 3.0.0-beta.1. Output:

image

@ccorcos

This comment has been minimized.

Copy link
Author

commented Jul 7, 2018

Hmm yeah, that's no good!

@ccorcos

This comment has been minimized.

Copy link
Author

commented Nov 14, 2018

Just tried this in Electron 3.0.9 and its still an issue and app.commandLine.appendSwitch('disable-mojo-local-storage'); does not fix the problem either. Any ideas @bpasero?

@ccorcos

This comment has been minimized.

Copy link
Author

commented Nov 15, 2018

@deepak1556 any idea what happened to this command line option to turn off mojo?

@bpasero

This comment has been minimized.

Copy link
Contributor

commented Nov 17, 2018

@ccorcos disable-mojo-local-storage will not work anymore starting with Electron 3.0.x due to the Chrome version used within.

@deepak1556

This comment has been minimized.

Copy link
Member

commented Nov 19, 2018

Seems like the localstorage limit is not completely removed, during commit operation the size constraint is still maintained at 5MB.

@deepak1556 deepak1556 changed the title LocalStorage silently failing Remove localstorage limit Nov 19, 2018

@SwiTool

This comment has been minimized.

Copy link

commented Feb 28, 2019

I checked that electron v4.0.4 is ok
image

Sorry but you failed, Array(1000).fill(1)/toString() gives NaN as Array(1000).fill(1).toString() gives 1,1,1,1,1 1000 times.
I still have the issue :
image

Using electron 4.0.6

@richjun

This comment has been minimized.

Copy link

commented Feb 28, 2019

I checked that electron v4.0.4 is ok
image

Sorry but you failed, Array(1000).fill(1)/toString() gives NaN as Array(1000).fill(1).toString() gives 1,1,1,1,1 1000 times.
I still have the issue :
image

Using electron 4.0.6

You're right.
It's my mistake.

@jacobq

This comment has been minimized.

Copy link
Contributor

commented May 22, 2019

I think the test code used above is misleading. (In fact, so is the test I originally contributed). I found that within a script I can call setItem and getItem back-to-back for as large of a size as I like (provided there is enough memory and the electron version being used includes #15596). e.g.

localStorage.setItem('junk', new Array(50*1024*1024/2).fill('x').join('')); console.log(localStorage.getItem('junk').length)

However, the data isn't necessarily persisted. If the getItem isn't run back-to-back (in the same closure / context / whatever) -- e.g. if it's run from the dev tools console after setItem finishes -- then it will return null or "".

As a more thorough test, I've forked the quick start app and scripted it to run a series of tests separated by setTimeout. You can view the code at jacobq/electron-quick-start#storageLimit.

For current versions (5.0.1 and 6.0.0-beta.4) the limit appears to be about 21176304 bytes (i.e. 20MiB + about 1% grace).

@jacobq jacobq referenced this issue May 22, 2019

Merged

fix: bypass DOM storage quota #15596

5 of 5 tasks complete
@ccorcos

This comment has been minimized.

Copy link
Author

commented May 28, 2019

Hmm. That's weird because localStorage is supposed to be synchronous

@jacobq

This comment has been minimized.

Copy link
Contributor

commented May 29, 2019

Hmm. That's weird because localStorage is supposed to be synchronous

I'd have to review the spec again, but I think it's synchronous from the perspective of the script. That is, calling setItem then getItem must give back the new value. However, I don't think the underlying structures are required to be, so if something goes wrong (e.g. disk full, limit exceeded, etc.) then future calls to getItem may differ.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.