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

NewPacket.header.len() is required for parsing, do not clear the vector #11

Merged
merged 1 commit into from Jun 20, 2017

Conversation

Projects
None yet
2 participants
@boxofrox
Contributor

boxofrox commented Jun 19, 2017

See rust-lang/rust#42610 for details.

I thought this bug was likely an optimization error, but turns out the glitch doesn't occur unless the program is running at full throttle with release target optimizations.

If a TCP packet ends with 4-byte MySQL packet header, and the next TCP packet is slow to arrive, then the MySQL parser will:

  1. Read the 4-byte header, decompose the bytes into length and sequence id, and clear the header vector.
  2. Follow-up with an attempt to copy bytes into data vector, but there are no bytes since that packet hasn't arrived, yet.
  3. Attempt one more parse which...
    1. find zero bytes in the data vector (still waiting for another TCP packet), and
    2. finds zero bytes in the header vector (header vector is empty), ...
  4. ...and then proceeds to read subsequent bytes as header info rather than data info.
NewPacket.header.len() is required for parsing, do not clear the vector
See rust-lang/rust#42610 for details.

Signed-off-by: Justin Charette <charetjc@gmail.com>

@boxofrox boxofrox force-pushed the boxofrox:fix/parsing-desync branch from 0f277a5 to 306e841 Jun 19, 2017

@blackbeam blackbeam merged commit 40d572a into blackbeam:master Jun 20, 2017

@blackbeam

This comment has been minimized.

Owner

blackbeam commented Jun 20, 2017

Thanks!

@blackbeam

This comment has been minimized.

Owner

blackbeam commented Jun 20, 2017

I'll try to find time to publish today together with remaining documentation for #10

@boxofrox

This comment has been minimized.

Contributor

boxofrox commented Jun 20, 2017

Cheers! 😁

@blackbeam

This comment has been minimized.

Owner

blackbeam commented Jun 20, 2017

Published as v0.10.1

@boxofrox boxofrox deleted the boxofrox:fix/parsing-desync branch Jun 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment