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

fixed createListeningPoint in case of IOException #175

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fre42
Copy link
Contributor

@fre42 fre42 commented Feb 20, 2018

We have a scenario where binding an IP address fails with IOException in createListeningPoint. The reason for that is that the network device with the given IP address is not there for some reason.
Our application has a mechanismn to retry binding after some time repeatedly because the missing network device will be added after some time and the next try to bind may be successful.
This will not work with the current implementation.
I've found out that the next try to call createListeningPoint() will not fail anymore although the network device is still not there and the IP address is therefore still not bindable. Even in the case where the IP address is bindable on the second try, a bind will not take place and we will end up in a listening point that does not work.
This happens because the implementation of createListeningPoint() stores the key for the listening point to a map called listeningPoints before the bind takes place (in messageProcessor.start()).
An IOExecption will be thrown but the map entry will not be removed. The next time createListeningPoint is called, it will find this map entry to returns it without starting the message processor.

My suggested fix is to store the listening point key to the map after having successfully called the messageProcessor.start() method to avoid having an orphaned entry in the listening points map.

@fre42
Copy link
Contributor Author

fre42 commented Feb 20, 2018

I know that signing the CLA is still missing. I'm still waiting for my company to give me the allowance. This will hopefully happen soon.

@gsaslis
Copy link
Contributor

gsaslis commented Feb 23, 2018

@fre42 no problem - plz ping back when signed ; )

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