Don't try to send self address when connecting as read-only #126
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When connecting as readonly node to the cluster with encryption enabled
the _onOutgoingMessageReceived() method tried to unconditionally send
_selfNode.address after an exchange of random keys. Since the
_selfNode.address is None following exception was raised:
```python
Exception in thread Thread-1:
Traceback (most recent call last):
File "/usr/lib/python2.7/threading.py", line 801, in __bootstrap_inner
self.run()
File "/usr/lib/python2.7/threading.py", line 754, in run
self.__target(*self.__args, **self.__kwargs)
File "/usr/local/lib/python2.7/dist-packages/pysyncobj/syncobj.py", line 505, in _autoTickThread
self._onTick(self.__conf.autoTickPeriod)
File "/usr/local/lib/python2.7/dist-packages/pysyncobj/syncobj.py", line 617, in _onTick
self._poller.poll(timeToWait)
File "/usr/local/lib/python2.7/dist-packages/pysyncobj/poller.py", line 97, in poll
self.__descrToCallbacks[descr](descr, eventMask)
File "/usr/local/lib/python2.7/dist-packages/pysyncobj/tcp_connection.py", line 174, in __processConnection
self.__onMessageReceived(message)
File "/usr/local/lib/python2.7/dist-packages/pysyncobj/transport.py", line 455, in _onOutgoingMessageReceived
conn.send(self._selfNode.address)
AttributeError: 'NoneType' object has no attribute 'address'
```
|
Could you write a unit test on this case (in test_syncobj.py)? |
|
Also I've got another question about encryption. Why the TcpConnection.encryptor is reset on disconnect? It makes it impossible for nodes to reconnect if one of the partners was disconnected/restarted/not accessible, because next time it will try to connect without encryption and remote side will drop such connections. |
|
Disconnect works as a destructor (for cases when garbage collector is disabled and only reference counting works). May be there is a bug, there are only few tests for encryption (was not a priority use case). You can write a test and fix it in separate PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When connecting as readonly node to the cluster with encryption enabled the _onOutgoingMessageReceived() method tried to unconditionally send _selfNode.address after an exchange of random keys.
Since the _selfNode.address is None, the following exception was raised: