[WIP] Fix caught callbacks #223
Conversation
Fix doubled callbacks, called once to handle results and then a second time to handle an error thrown in the callback Does not compile! Yet to do FS.ts Need to revisit: * AsyncMirror.Create * IndexedDBROTransaction.get * IndexedDBRWTransaction.put, .del * IndexedDBStore.clear * IndexedDBFileSystem.create * OverlayFS.Create * ZipFS.Create * ZipFS._computeIndex
https://github.com/1j01/BrowserFS/tree/2da9e4eba77664f3fce75ea1c21cc31aaa08c846 Part of WIP pull request jvilk/BrowserFS#223 (package.json / package-lock.json not updated, but it's now linked with a version near 2.0.0-beta)
|
Sorry for letting this sit for a month! These changes look good to me. Should I merge this in, or wait for you to finish your pass through FS? |
src/backend/ZipFS.ts
Outdated
| err = _err; | ||
| zipTOC = _zipTOC; | ||
| }; | ||
| ZipFS._computeIndexResponsive(data, index, cdPtr, cdEnd, callbackWrapper, cdEntries, eocd); |
There was a problem hiding this comment.
I'm relying on _computeIndexResponsive calling back synchronously, but it's not actually always synchronous. If it was always async, I think the callback in try would be okay, but would warrant a comment like // MUST be asynchronous to avoid a double callback in _computeIndexResponsiveTrampoline
There was a problem hiding this comment.
I see; yeah, it's not always synchronous. There's a dumb hacky way to get around the issue, but I hesitate to suggest it, as it'll make the code nasty.
I can handle refactoring / fixing this specific issue, so please revert this change that you've made.
|
Been a long time, lots of conflicts - please let me know if you are still interested in pursuing this or if we should close, thanks. |
|
Closing for now. Can always re-open; just post a comment. |
Fixing doubled callbacks, called once to handle results and then a second time to handle an error thrown in the callback (like described here)
Yet to do: FS.ts
Need to revisit:
Fixes #221