ether/etherpad-lite — pull request #2092 by webzwo0i (2014-02-20 18:25:20+00:00)
Title: Fix crash with queued messages

if a disconnect msg is received, handleDisconnect is called and deletes the session
if a message is received, handleMessage is called and a lot of callbacks are fired to handle the message.
at the beginning of handleMessage is a check that ensures the session exists.

if the server is under heavy i/o load, callbacks get delayed and it can happen that a disconnect message is handled before all callbacks that depend on the existence of the session.

a similar patch was applied in the past (https://github.com/ether/etherpad-lite/pull/1025) but it was not enough. eplite will crash if I use the bug in https://github.com/ether/etherpad-lite/pull/2091 to insert a lot of changesets up to the point, where ueberDB's internal cache size is reached. suppose the default size of 1000 is used. at about 1000 revisions, the garbage collector kicks in and deletes half of the cache, oldest entries first. (say revisions 1 upto 500). I submit a new cs and the cache is filled with 500 new entries, and gc starts again and deletes revision 501 upto 1000.
at this point ueberDBs cache is completely unused and every request that is necessary to rebase the cs to the current head will hit the database (as if cache size was 0) and the result is a lot of i/o.

now I submit some changesets with baseRev:0 and a disconnect. some will be queued in a channel and when they are processed the session is gone. for others the check at the beginning of handleMessage will be okay, but when the finalHandler is processed the session is gone.

I think it may be possible to exactly predict all the places where this can happen, but my understanding of nodejs' event system is not good enough for this. I was thinking it can only happen in async-functions, that queue processing with nextTick/setTimeout/setImmediate, so every code path that uses those function must ensure that the session is present.

thats why I left a "TODO" in the patch. At a later point, instead of checking in so many different places if the session (or some object keys in the message etc) are still present it would be easier to check this only at one place (in the beginning of handleMessage), make a copy of the session, and hand this copy over to all callbacks that need it. every message that had a proper session when they arrived will always have it during later processing, no matter how much delay is in between.
what do you think?