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

Broken FlowControlHandler when auto-read is disabled (caused by #10326) #10464

Closed
ursaj opened this issue Aug 10, 2020 · 4 comments
Closed

Broken FlowControlHandler when auto-read is disabled (caused by #10326) #10464

ursaj opened this issue Aug 10, 2020 · 4 comments

Comments

@ursaj
Copy link
Contributor

ursaj commented Aug 10, 2020

Expected behavior (v.4.1.44)

In my handler:

ctx.read()
    readComplete(ctx) // when no messages in network
        ctx.read() // multiple messages arrived simultaneously
            channelRead(ctx, msg) // single message
            // no more calls for channelRead or readComplete

Expected behavior (v.4.1.51+)

In my handler:

ctx.read()
    readComplete(ctx) // when no messages in network + reset inner counter 
        ctx.read() // multiple messages arrived simultaneously
            channelRead(ctx, msg) // single message
            // no more calls for channelRead or readComplete

or

ctx.read()
    // no readComplete(ctx) when no messages in network
    channelRead(ctx, msg) // when message actually arrived
        ctx.read()
            channelRead(ctx, msg) // single message
            readComplete() // only when no messages in network and no more messages requested from flow control handler

Note, there are a lot of corner-cases to think of:

  • inner counter
  • buffer state
  • readComplete called
  • stack overflow, when ctx.read is called from channelRead or readComplete
  • don't do any inner re-scheduling (if possible)
  • etc...

Actual behavior

ctx.read() // inner counter = 1
    readComplete(ctx)
        ctx.read() // inner counter = 2
            channelRead(ctx, msg) // expected, ok
            channelRead(ctx, msg) // unexpected, overflow => big BOOM!

Root cause

#10326

Netty versions

4.1.44
4.1.51

JVM version (e.g. java -version)

% java -version
openjdk version "1.8.0_242"
OpenJDK Runtime Environment (AdoptOpenJDK)(build 1.8.0_242-b08)
OpenJDK 64-Bit Server VM (AdoptOpenJDK)(build 25.242-b08, mixed mode)

OS version (e.g. uname -a)

% uname -a
Darwin MBPWORK 19.5.0 Darwin Kernel Version 19.5.0: Tue May 26 20:41:44 PDT 2020; root:xnu-6153.121.2~2/RELEASE_X86_64 x86_64
@normanmaurer
Copy link
Member

@ursaj so do you say that this is a regression?

@ursaj
Copy link
Contributor Author

ursaj commented Aug 10, 2020

exactly

@normanmaurer
Copy link
Member

@ursaj got it... sounds like we better revert this one. thanks for the report

normanmaurer added a commit that referenced this issue Aug 11, 2020
Motivation:

This reverts commit b559711 due regression introduced by it.

Modification:

Revert commit

Result:

Fixes #10464
@normanmaurer
Copy link
Member

Done...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants