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

x/mobile: app.pump() will eventually crash #13230

Closed
db47h opened this issue Nov 13, 2015 · 4 comments

Comments

Projects
None yet
3 participants
@db47h
Copy link

commented Nov 13, 2015

The i and j counters are never reset if the buffer is never expanded and will eventually reach MAXINT. This may seem overzealous since on a 32 bits system, this can take from a couple of months to over a year depending on the amount of events. Some applications like public information screens can however be expected to run for longer than that.

@crawshaw

This comment has been minimized.

Copy link
Contributor

commented Nov 13, 2015

That's fair. Note to anyone who wants to fix this: there's a second copy of the code in x/exp/shiny/driver/internal/pump/pump.go. Ideally we would start by fixing that, and then copying that code over as x/mobile/internal/pump/pump.go, so that it's easier to keep them in sync in the future.

@crawshaw crawshaw added this to the Unreleased milestone Nov 13, 2015

@db47h

This comment has been minimized.

Copy link
Author

commented Nov 13, 2015

Sorry, I know this is border line OCD, I'll submit a CL. Regarding the fix, does anyone have a better idea than the obvious

    // ...
    // Allocate a bigger buffer if necessary.
    if i+len(buf) == j {
        // ...
    }
    buf[j&mask] = e
    j++
    if j == int(^uint(0)>>1) {
        i, j = i&mask, j-i+(i&mask)
    }
}

?

@crawshaw

This comment has been minimized.

Copy link
Contributor

commented Nov 13, 2015

Most of the modulo arithmetic survives through overflow. The only part I haven't thought through carefully is i+len(buf) == j.

Can you first construct some example values where this code fails?

@db47h

This comment has been minimized.

Copy link
Author

commented Nov 14, 2015

Woops, forget it, I got modulo (%) mixed up with the fast modulo performed with & mask. It works. i + len(buf) == j also works if only j has overflowed.

@db47h db47h closed this Nov 14, 2015

@golang golang locked and limited conversation to collaborators Nov 16, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.