-
Notifications
You must be signed in to change notification settings - Fork 415
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
Few critical bugs are fixed #97
base: master
Are you sure you want to change the base?
Changes from all commits
315577e
ba4cf39
7730430
3d0112f
3fcc194
33a0052
637937a
dbeabca
15e9c8f
a4ffae8
d8ed136
3f7e84c
4d88593
219ba41
4027172
6066d6e
3427a77
ef31e53
f308dc9
37435ee
f492ba8
222365b
f8c2dcf
a4b2eda
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,10 +46,14 @@ class TcpConnection { | |
private volatile long lastWriteTime, lastReadTime; | ||
private int currentObjectLength; | ||
private final Object writeLock = new Object(); | ||
private final int objectBufferSize; | ||
private final int lengthLength; | ||
|
||
public TcpConnection (Serialization serialization, int writeBufferSize, int objectBufferSize) { | ||
this.serialization = serialization; | ||
writeBuffer = ByteBuffer.allocate(writeBufferSize); | ||
this.objectBufferSize = objectBufferSize; | ||
lengthLength = serialization.getLengthLength(); | ||
writeBuffer = ByteBuffer.allocate(Math.max(writeBufferSize, lengthLength + objectBufferSize)); | ||
readBuffer = ByteBuffer.allocate(objectBufferSize); | ||
readBuffer.flip(); | ||
} | ||
|
@@ -199,8 +203,21 @@ public int send (Connection connection, Object object) throws IOException { | |
SocketChannel socketChannel = this.socketChannel; | ||
if (socketChannel == null) throw new SocketException("Connection is closed."); | ||
synchronized (writeLock) { | ||
int start = writeBuffer.position(); | ||
int lengthLength = serialization.getLengthLength(); | ||
|
||
//wait while buffer has enough free space. | ||
//thats because message length will be put only after the whole message will be serialized | ||
while (writeBuffer.capacity() < writeBuffer.position() + lengthLength + objectBufferSize) { | ||
try { | ||
Thread.sleep(10); | ||
} catch (InterruptedException e) { | ||
break; | ||
} | ||
writeToSocket(); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't just sleep the writing thread, this would have to be a feature, disabled by default. The right way to handle this is to not send data so fast or to use a larger buffer. Comment should be "wait until buffer". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you pass 0 as value of objectSize, feature will be disabled. We can set 0 as default value. Concerning "The right way", it's the matter of dispute. If so user should select unique buffer size for each system installation on different computers, in different configurations and so on. Or make buffer too large to avoid problems that are very rare. |
||
|
||
// Leave room for length. | ||
int start = writeBuffer.position(); | ||
try { | ||
// Leave room for length. | ||
writeBuffer.position(writeBuffer.position() + lengthLength); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use the write buffer size that was passed in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because bufferSize >= lengthLength + objectSize must be true in all cases.
If it is not so, user has entered incorrect values. We can react on that two ways:
first - throw an exception, second - use max(bufferSize, lengthLength + objectSize).
We have chosen second one, but we can change to exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I prefer an exception to using a buffer size other than what was specified.