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

Storage Compact can break storage #2009

Closed
gfwilliams opened this issue May 28, 2021 · 4 comments
Closed

Storage Compact can break storage #2009

gfwilliams opened this issue May 28, 2021 · 4 comments

Comments

@gfwilliams
Copy link
Member

gfwilliams commented May 28, 2021

On Bangle.js if you write a bunch of stuff to storage and then compact, it can actually corrupt storage.

However, it doesn't happen all the time...

@jumjum123
Copy link
Contributor

jumjum123 commented May 28, 2021

Same happens on ESP32 from time to time

@gfwilliams
Copy link
Member Author

gfwilliams commented May 28, 2021

Interesting - thanks! If anyone can come up with a way to reproduce it reliably (eg specific calls to require("Storage").write then I'd love it if you could post up.

As far as I can tell this only really happens if you have a 'corrupt' filename shown in the IDE's storage list or require("Storage").list() - but it's a twofold issue:

  • .compact shouldn't completely fail when faced with a dodgy file
  • We shouldn't get messed up files in the first place!

gfwilliams added a commit that referenced this issue Jun 10, 2021
@gfwilliams
Copy link
Member Author

gfwilliams commented Jun 10, 2021

You could reproduce one issue with:

const Storage = require('Storage');
Storage.eraseAll();
Storage.write('a', new Uint8Array(500));
Storage.write('code', 'function hello() { print("Hello1"); }');
Storage.write('code', 'function hello() { print("Hello2"); }');
Storage.write('code', 'function hello() { print("Hello"); }');
eval(Storage.read('code'));
trace(hello);
hello();
Storage.compact();
hello();
trace(hello);
eval(Storage.read('code'));
trace(hello);

In that case, the function 'hello' ended up pointing to the wrong place after the compact - the commit above fixes that.

However, I'm not sure this entirely fixes the problem. You'd hope that this code would flex the compaction algorithm pretty well, and it works totally fine, repeatedly:

function rint(l) { return Math.floor(Math.random()*l); }
function rando(l) { return new Uint8Array(l).map(x=>Math.random()*256); };

const Storage = require('Storage');
Storage.eraseAll();
for (var i=0;i<10;i++) Storage.write('a'+i, new Uint8Array(rint(5000)));
var d = E.toString(rando(15000));
Storage.write('data', d);
if (Storage.read('data')!=d) throw "Data not correct 1";
for (var i=0;i<10;i++) Storage.write('b'+i, new Uint8Array(rint(5000)));
// now overwrite stuff
for (var i=0;i<10;i++) Storage.write('a'+i, new Uint8Array(rint(5000)));
for (var i=0;i<10;i++) Storage.write('b'+i, new Uint8Array(rint(5000)));
// data should still be fine - shouldn't be touched
if (Storage.read('data')!=d) throw "Data not correct 2";
// compact
Storage.compact();
// everything moved, but we hope data is still fine
if (Storage.read('data')!=d) throw "Data not correct 3";
print("ok");

And the only time I have hit issues is once when I have done a compact after I have seen a broken-looking filename in Storage.list.

However I am unable to reproduce the broken filename or the compaction issues.

So I'll leave this open for a while in case someone can come up with sample code that can corrupt storage in some way, but if not I think it's safe to close.

@gfwilliams
Copy link
Member Author

gfwilliams commented Jan 26, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants