Skip to content
This repository has been archived by the owner on Jun 8, 2020. It is now read-only.

Defect: NettyStreamingService.disconnect() method does not shutdown a… #225

Closed
wants to merge 0 commits into from

Conversation

pchertalev
Copy link
Contributor

…ll threads - FIXED

  • FIX NullPointerException on first "partial"

Looks like it is fix of bugs: #145 #35

Copy link
Collaborator

@Flemingjp Flemingjp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NettyStreamService - What is the purpose of creating a new instance of an NioEvent?

@badgerwithagun
Copy link
Collaborator

Pretty sure #191 resolves this. There were quite a few edge cases it took a while to iron out.

@Flemingjp
Copy link
Collaborator

Recommdation would be to close this PR?

@badgerwithagun
Copy link
Collaborator

I'm going to try merging this locally to see the differences in place. WIll report back

@badgerwithagun
Copy link
Collaborator

OK, so it's a mixed bag.

This is a good PR. If it's merged before #191 it would be beneficial on its own. However, it would be a pain to fix up #191 again so it merges, and that would screw up the cleanup proposal I put in #224.

My preference would be to get #191 merged first, then follow up with this when we can make a choice about the event group thread count question.

@Flemingjp
Copy link
Collaborator

Ok, this should be put on hold until we can merge #191

@pchertalev
Copy link
Contributor Author

pchertalev commented Oct 8, 2018

No problemo. I will resolve conflicts after merge #191

@Flemingjp
Copy link
Collaborator

@pchertalev #191 has now been merged. If you could test to see if the problem still exits, and requires this PR that'd be great

@pchertalev pchertalev closed this Oct 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants