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

[LOGMGR-314] SocketHandlerTests produce BindException: Address already in use #374

Merged
merged 1 commit into from
Jan 20, 2023

Conversation

kstekovi
Copy link
Contributor

@kstekovi kstekovi commented Jan 5, 2023

@jamezp
Copy link
Member

jamezp commented Jan 5, 2023

We might be best to just pass 0 to the port and then change the SimpleServer to return the port it used instead. Or we should use the loop to find the open port in the getPort() rather than the tests themselves.

@kstekovi kstekovi force-pushed the LOGMGR-314 branch 2 times, most recently from 5b2cba0 to 7b7e196 Compare January 9, 2023 09:09
@kstekovi
Copy link
Contributor Author

kstekovi commented Jan 9, 2023

Hi @jamezp Thank you for your advice to use 0 which will use the free port automatically. This is great for most test is this TC. But unfortunately this approach cannot be used for testProtocolChange and testTcpReconnect because one handler and two servers are used in the test. When is created second server for test it must use same port as the first server in the test. So I added sleep to give OS some extra time to release port used by the first server and avoid java.net.BindException: Address already in use

the SocketHandler cannot be created with 0 as ServerSocket and exact port must be specified. With 0 it produce IllegalStateException: An address and port greater than 0 is required.

@kstekovi
Copy link
Contributor Author

Hi @jamezp could you please take a look?

Assert.assertEquals("Test TCP handler", msg);
}
// wait until the OS really release used port. https://issues.redhat.com/browse/LOGMGR-314
Thread.sleep(50);
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed? It feels like a guess at best TBH and feels fragile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think too this should not be here. But I didn't find way how to safely verify a port is free. All ways which i found are similar to:

try (Socket ignored = new Socket("localhost", port)) {
        return false;
    } catch (ConnectException e) {
        return true;
    }

And we don't have guarantees after end of try with resources or after close method is a port really free. Sometimes isn't - especially when the CPU is utilized to 100% and these tests are started.

@jamezp
Copy link
Member

jamezp commented Jan 20, 2023

Upstream #377

@jamezp jamezp merged commit 03a72b7 into jboss-logging:2.1 Jan 20, 2023
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

Successfully merging this pull request may close these issues.

2 participants