turuslan / HackTimer

MIT License
491 stars 93 forks source link

Race condition that lead to bad fakeId generation #12

Closed Mwyann closed 8 years ago

Mwyann commented 8 years ago

Hi, I'm developing an HTML5 game (port of an old game, see http://ydkjs.eu or https://github.com/Mwyann/ydkjs) and i'm using this library to prevent the game from pausing when the tab loses focus. But I'm using a lot of timeouts and frequently the getFakeId would give the same ID to a couple of timeouts, leading to bugs obviously (callbacks not called properly).

For now I got around the problem by generating random fakeIds, that's maybe not very clean and maybe there's a 0.00001% chance of collision but that's enough for me :)

        function getFakeId () {
            var newId;
            do {
                newId = Math.floor(Math.random()*maxFakeId);
            } while (fakeIdToCallback.hasOwnProperty (newId));
            return newId;
        }

If there's a better solution, I'd be glad to know it.

turuslan commented 8 years ago

@Mwyann, thanks for reporting. Probably changes of commit e11f3ed2292062ee1e1aafe3510802a889ea6f96 caused this, because it allowed last fake id to be reused if it became free. So I suppose, that function may be modified in such a way

do {
    if (lastFakeId == maxFakeId) {
        lastFakeId = 0;
    } else {
        lastFakeId ++;
    }
} while (fakeIdToCallback.hasOwnProperty (lastFakeId));

so it will always increase fake id to prevent collisions in short-term (about 231 - 1 ids).

turuslan commented 8 years ago

Tomorrow I'll make pull request of fix.

Mwyann commented 8 years ago

I updated my code with your fix and it seems to work better apparently, as good as my random method, but yours looks "nicer". I'll tell you if I see strange behavious again.

turuslan commented 8 years ago

Merged pull request https://github.com/turuslan/HackTimer/pull/14. Thanks again for your help.