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

Improvements in socket connect to server #962

Closed
olivierayache opened this issue Jan 8, 2020 · 2 comments · Fixed by #964
Closed

Improvements in socket connect to server #962

olivierayache opened this issue Jan 8, 2020 · 2 comments · Fixed by #964

Comments

@olivierayache
Copy link
Contributor

olivierayache commented Jan 8, 2020

When you use a custom socketFactory this code can be buggy:

if (!socket.isBound()) {
  InetSocketAddress addr = new InetSocketAddress(dnsResolver.resolve(uri), this.getPort());
  socket.connect(addr, connectTimeout);
}

Indeed socket might already be bound by socketFactory and isBound() will return true. A better version could be

if (!socket.isConnected()) {
  InetSocketAddress addr = new InetSocketAddress(dnsResolver.resolve(uri), this.getPort());
  socket.connect(addr, connectTimeout);
}

This issue is linked to the solution I mentionned in issue #814

@olivierayache olivierayache changed the title Improvments in socket connect to server Improvements in socket connect to server Jan 8, 2020
@marci4
Copy link
Collaborator

marci4 commented Jan 8, 2020

Hello,
Could you provide additional steps to repeat?
Can this issue be tested somehow?

And if you, would you be so kind an open a pull request?

Thx a lot
Marcel

@olivierayache
Copy link
Contributor Author

Ok I will open a pull request.

Olivier

olivierayache added a commit to olivierayache/Java-WebSocket that referenced this issue Jan 9, 2020
olivierayache added a commit to olivierayache/Java-WebSocket that referenced this issue Jan 9, 2020
@marci4 marci4 added this to the Release 1.4.1 milestone Jan 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants