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

[UNDERTOW-2241][UNDERTOW-2247][UNDERTOW-2248] Handle write timeout properly so no chunk errors occur #1449

Closed

Conversation

fl4via
Copy link
Member

@fl4via fl4via commented Mar 24, 2023

@fl4via fl4via changed the title [UNDERTOW-2241][UNDERTOW-2247][UNDERTOW-2248] Handle write timeout properly to no chunk errors occur [UNDERTOW-2241][UNDERTOW-2247][UNDERTOW-2248] Handle write timeout properly so no chunk errors occur Mar 24, 2023
@fl4via fl4via added bug fix Contains bug fix(es) under verification Currently being verified (running tests, reviewing) before posting a review to contributor waiting peer review PRs that edit core classes might require an extra review waiting CI check Ready to be merged but waiting for CI check labels Mar 24, 2023
@fl4via fl4via requested a review from ropalka March 24, 2023 02:55
@fl4via fl4via force-pushed the UNDERTOW-2241_2247_2248_master branch 4 times, most recently from 3c32a4a to 03b4de4 Compare March 24, 2023 03:17
@fl4via fl4via added waiting PR update Awaiting PR update(s) from contributor before merging failed CI Introduced new regession(s) during CI check and removed waiting CI check Ready to be merged but waiting for CI check labels Mar 24, 2023
@fl4via
Copy link
Member Author

fl4via commented Mar 24, 2023

This failed Windows CI. I'll have to do an investigation of the reason for the failure.

@fl4via fl4via force-pushed the UNDERTOW-2241_2247_2248_master branch 6 times, most recently from 3a2b7fe to c4cd8da Compare April 23, 2023 06:38
@fl4via fl4via removed the waiting peer review PRs that edit core classes might require an extra review label Apr 23, 2023
@fl4via
Copy link
Member Author

fl4via commented Apr 23, 2023

Everything indicates the failure in the fix UNDERTOW-2247 is not fully correct. The failure in Windows is a race condition and it requires further work. I'll get back to this after my PTO, on May.

@fl4via
Copy link
Member Author

fl4via commented Apr 23, 2023

I'm closing for now. Will reopen when I get back to it.

@fl4via fl4via closed this Apr 23, 2023
@fl4via fl4via reopened this May 17, 2023
Signed-off-by: Flavia Rainone <frainone@redhat.com>
…and clear the allocated buffer to prevent NPEs when ClosedChannelExceptions and broken pipe SocketExceptions occur.

Those NPEs can occur at the points where the buffer is closed and it is already null because the buffer was cleared by an inner recursive call as an attempt to deal with the exception.
The broken pipe can occur when a browser tab is closed during the response writing process.

Signed-off-by: Flavia Rainone <frainone@redhat.com>
…reset expireTime to -1, disabling future checks for write timeout at timeoutCommand runnable

Signed-off-by: Flavia Rainone <frainone@redhat.com>
…ore closing the connection to give a chance to outer channels wrap up. At ChunkedStreamSinkConduit create the last chunk and attempt to flush before truncating writes.

Signed-off-by: Flavia Rainone <frainone@redhat.com>
@fl4via fl4via force-pushed the UNDERTOW-2241_2247_2248_master branch 2 times, most recently from 164b176 to d6d724a Compare June 7, 2023 04:34
…ring and, instead, just fail the test if there is no header

Signed-off-by: Flavia Rainone <frainone@redhat.com>
…s failed on unrelated PR CIs we know it is a preexisting intermitent failure

Signed-off-by: Flavia Rainone <frainone@redhat.com>
@fl4via fl4via force-pushed the UNDERTOW-2241_2247_2248_master branch from d6d724a to 177dd0e Compare June 7, 2023 05:55
@fl4via fl4via added next release This PR will be merged before next release or has already been merged (for payload double check) waiting CI check Ready to be merged but waiting for CI check and removed waiting PR update Awaiting PR update(s) from contributor before merging under verification Currently being verified (running tests, reviewing) before posting a review to contributor failed CI Introduced new regession(s) during CI check labels Jun 7, 2023
@fl4via fl4via closed this Jun 7, 2023
@fl4via
Copy link
Member Author

fl4via commented Jun 7, 2023

Something odd happened wiht this PR that, even after 10 minutes of having pushed my branch, the PR was not updated, but I could see the branch was updated by following the link from the PR page to the branch.
I ended up having to close it and create a new one, unfortunately :-(

@fl4via fl4via added duplicate Duplicates other pull request(s) and removed bug fix Contains bug fix(es) next release This PR will be merged before next release or has already been merged (for payload double check) waiting CI check Ready to be merged but waiting for CI check labels Jun 7, 2023
@fl4via
Copy link
Member Author

fl4via commented Jun 7, 2023

The new PR is #1489

@fl4via
Copy link
Member Author

fl4via commented Jun 7, 2023

I had to recreate a new PR for the same reason :-(
New PR is #1490

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate Duplicates other pull request(s)
Projects
None yet
1 participant