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

[XNIO-385] Ensure that the close method waits until the server key ca… #248

Closed
wants to merge 1 commit into from

Conversation

yersan
Copy link
Contributor

@yersan yersan commented Jul 7, 2021

…ncellation releases the resources

Jira issue: https://issues.redhat.com/browse/XNIO-385
3.x PR: #249

Copy link
Member

@dmlloyd dmlloyd left a comment

Choose a reason for hiding this comment

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

Looks good, but it should be tested in integration with a WildFly build before merging.

@yersan
Copy link
Contributor Author

yersan commented Jul 9, 2021

There are issues with the wildfly test suite with this upgrade. This test gets stuck:
https://github.com/wildfly/wildfly-core/blob/e586ab8c23763d54f11be6db75de894b126e5c1f/protocol/src/test/java/org/jboss/as/protocol/mgmt/RemoteChannelManagementTestCase.java#L142

Removing the change on NioSocketStreamConnection.java makes it pass. I've been unable to understand the reason why it gets stuck. It is a random error.

@dmlloyd the fact of being this specific change tells you something about the reason?

This is the output with TRACE level: https://gist.github.com/yersan/88729bd0b7fdedf76c3898321052a0c8

@yersan
Copy link
Contributor Author

yersan commented Jul 12, 2021

@dmlloyd WildFly and WildFly core TS pass if we don't apply the change on the NioSocketStreamConnection.

What's your opinion if we apply the change only for NioTcpServer.java , this is the one that causes the initial issue. Or, alternatively, allow the client to explicitly close the resources passing the block argument, so we can use in our testsuite and workaround the original issue

@dmlloyd
Copy link
Member

dmlloyd commented Jul 12, 2021

@dmlloyd WildFly and WildFly core TS pass if we don't apply the change on the NioSocketStreamConnection.

What's your opinion if we apply the change only for NioTcpServer.java , this is the one that causes the initial issue. Or, alternatively, allow the client to explicitly close the resources passing the block argument, so we can use in our testsuite and workaround the original issue

Let's just apply it to NioTcpServer. Letting the user specify an argument like this will probably lead to hangs and other problems.

@yersan
Copy link
Contributor Author

yersan commented Jul 12, 2021

Ok, thank you, I'll update the PR and paste here the links of the Jobs

@yersan
Copy link
Contributor Author

yersan commented Jul 14, 2021

David, as you commented on https://issues.redhat.com/browse/WFCORE-5386, this change is a little tricky, sadly, I am still getting random errors on RemoteChannelManagementTestCase, these are a couple of Jobs that test the integration with WildFly:

https://ci.wildfly.org/viewLog.html?buildId=263711&buildTypeId=WF_WildFlyCoreIntegrationExperiments
https://ci.wildfly.org/viewLog.html?buildId=263708&buildTypeId=WF_WildFlyCoreIntegrationExperiments

Other Jobs outside of the WildFly infrastructure
https://github.com/yersan/multi-repo-ci/actions/runs/1023137981
https://github.com/yersan/multi-repo-ci/actions/runs/1027071778
https://github.com/yersan/multi-repo-ci/actions/runs/1027078111

Sadly, due to the results and its random nature, we should not go with this patch, and we have to find an alternative to avoid hit it in our testsuite.

@dmlloyd If you agree and you have no other suggestions, feel free to close these PRs as an acknowledgment of the situation.

@dmlloyd
Copy link
Member

dmlloyd commented Aug 24, 2021

I don't have any other ideas for the moment, unfortunately.

@dmlloyd dmlloyd closed this Aug 24, 2021
@yersan yersan deleted the bugs/XNIO-385 branch August 30, 2021 09:28
@yersan yersan restored the bugs/XNIO-385 branch April 8, 2022 17:34
@yersan yersan deleted the bugs/XNIO-385 branch April 8, 2022 17:34
@yersan yersan restored the bugs/XNIO-385 branch October 4, 2023 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants