Vladsz83 commented on code in PR #12663:
URL: https://github.com/apache/ignite/pull/12663#discussion_r2738050325
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryIoSession.java:
##########
@@ -79,19 +79,19 @@ public class TcpDiscoveryIoSession {
/** */
private final Socket sock;
- /** */
+ /** Message writer. Access should be thread-safe. */
Review Comment:
Minor suggestion: Just '... Should be thread-safe.' Same below.
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryIoSession.java:
##########
@@ -246,7 +246,7 @@ <T> T readMessage() throws IgniteCheckedException,
IOException {
* @throws IgniteCheckedException If serialization fails.
* @throws IOException If serialization fails.
*/
- byte[] serializeMessage(TcpDiscoveryAbstractMessage msg) throws
IgniteCheckedException, IOException {
+ synchronized byte[] serializeMessage(TcpDiscoveryAbstractMessage msg)
throws IgniteCheckedException, IOException {
Review Comment:
maybe we should do something like:
```
synchronized(ses) {
ses.serializeMessage(msg);
}
```
and
```
synchronized(clientMsgWorker.ses) {
clientMsgWorker.ses.serializeMessage(msg)
}
```
where it is actually required. Server i/o doesn't need this synchronization
due to its nature. At least for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
