-
Notifications
You must be signed in to change notification settings - Fork 11
Concurrency issue - Posting from two different tabs windows does not work #139
Comments
I've done a little bit of investigation on this. It looks like the only way to share an SSB object (which I think is going to be the only way to solve the concurrency issue) is to make sure that all "new tab" operations (center-clicking things) are converted into window.open() calls so that we can access window.opener.SSB from the child tab. We will then need to keep a reference around to each of those child windows so that we can notify them if the parent window closes, because we will lose our SSB state and they will need to pause. This would also require that all references to the global SSB object are instead redirected to parent windows if we have them. It's doable, and indirection could be done synchronously, making it relatively easy to drop into place. It's not exactly easy, but it's doable. Now, the better solution, if we can swing it: Theoretically, it might also be possible for a parent window to pass each child window references to all other child windows so that if the parent window is closed it can elect one child window to be the holder of SSB. After closure, that child window would have to re-initialize SSB (because I don't think we can just pass the reference, and it wouldn't have any of the timers it needs for connection scheduling and such) and then proceed. This is arguably a more robust way to do it if we can pull it off, but it would require that all references to global SSB be converted into an asynchronous callback so that child windows would continue running without crashing while the newly-elected SSB holder reinitializes its SSB object. But to know if that's even realistic, I'll have to do some more testing. |
Looks like another option for getting around https://developer.mozilla.org/en-US/docs/Web/API/Window/opener Also appears to be supported in at least recent Firefox and Blink-based browsers, probably also WebKit-based. Good possibility IE does not support it (Edge should). More investigation needed. |
To use We could also use a variable in Web Workers and Service Workers wouldn't work well for this because we'd be limited to message passing which can't serialize functions, so we can't just pass a global singleton So I think our least fragile option is hooking @arj03 Any ideas? |
Seeing ssbc/ssb-db2#196 it would probably be a good idea to focus on this. I'll give it a look, I have been pretty busy with a bunch of things included this that should give a nice perf boost. |
I was expecting this to cause forks, but corrupt the database. So, yeah, this is a fairly high priority issue, then. |
Er, I was expecting it would not corrupt the database. |
I think it's basically the same as starting two programs that runs on the same file with different state in each program, so even if the file is atomic wrt. writes, then the state is not so it doesn't really add up in the end if you read it. |
Linking issues to make this a bit easier to navigate. I'm assuming all of these stem from the same underlying problem of the browser not sharing state: |
That is some really nice research you did there @KyleMaas! Sorry for not responding to this earlier. I think we need to first have a footgun where we use sessionstorage to see if other tabs are open and if so try to get the state window.opener.SSB. If that is not possible, like if you just open multiple tabs then just refuse to run and tell the user why. Secondly we could use the window.open like you explained to fix links so that window.opener.SSB works. It should be tested in browser.js, so that part seems like a quite small change. I tested the window.opener.SSB and it actually works pretty great! Is that something you could work on @KyleMaas? Just so we don't step on toes here :) |
Sure thing! I just didn't want to try to tackle it until we knew which route we wanted to go with. |
Sweet! |
So, my plan for this is as follows:
At each step, it should be fairly straightforward and testable. Where we really start running into unknowns is at (2). But this is going to require a lot of changes across pretty much the whole codebase (anywhere that accesses the |
Hmm I wonder if it would be possible do without the indirection. Another way would be to check if there is a window.opener.SSB in browser.js and use that instead of creating one. If you do it that way, are you sure that the SSB object will go away once the first window closed, there should now be an extra reference? And even if the firs window did go away, maybe that is an acceptable trace-off that makes this a lot easier and less error prone? |
I guess we can test that. But I think the indirection route would be more robust, because it would allow for a mutex lock gracefully degrade child windows if they couldn't find a parent instead of just crashing. |
Tested in both Chromium and Firefox, and if you create a reference in the child window, the SSB object is persisted even if the parent is closed. |
Nice, that is what I hoped for |
Interestingly, multiple child windows can hold their own references to the parent's SSB object even when all but one child window is closed. Which means, somehow, it's also carrying that object across process boundaries. Now, that said, the timers stop running when you close the parent window. Which means that nothing updates after you close the parent window, connections stop working, and presumably the write debouncing in AAOL would stop, leaving buffered writes unwritten. So...not perfect. To do it properly without indirection, we'd need to stop all of the child windows as soon as the parent window was closed. We'd have to add another layer on top of things, maybe with a broadcast channel. Which is possible, I guess, but feels less clean to me than a layer of indirection with a mutex to manage that and reinitialize as necessary. |
AFAICT it looks like it also stops responding to events from open connections once the parent window is closed. So, yeah, it looks like it works, until you try to actually do something with it. It brings the data over, but little of the functionality. |
Any further thoughts on this? |
If we can do the indirection without async I'm all for mutex. I think it would be okay that it said something like, oops parent window closed, reinitializing SSB and then things don't work until it finishes that. This way we could avoid changing everything to async. |
I'll see what I can do! |
Pretty sure we can close this now. |
If you open two different browser tabs (for example, to look at two different threads) and try to post to one, then switch to the other tab and post there, the messages from one will not show up on the other. It seems to behave as if you've forked your identity.
The text was updated successfully, but these errors were encountered: