fishy commented on code in PR #3131:
URL: https://github.com/apache/thrift/pull/3131#discussion_r2063970234
##########
lib/java/src/main/java/org/apache/thrift/transport/TServerSocket.java:
##########
@@ -122,6 +126,7 @@ public TSocket accept() throws TTransportException {
throw new TTransportException("Blocking server's accept() may not return
NULL");
}
TSocket socket = new TSocket(result);
+ socket.getConfiguration().deepCopy(configuration_);
Review Comment:
I think in general we want to propagate a single `TConfiguration` instance
across the stack, vs. copying. This is at least implied in the spec of
TConfiguration
(https://github.com/apache/thrift/blob/master/doc/specs/thrift-tconfiguration.md,
it mentioned "...we want to pass the instance by reference rather than by
value.").
That's also how I implemented it in go. see the NOTE part of the doc of
TConfiguration for some explanation:
https://pkg.go.dev/github.com/apache/thrift/lib/go/thrift#TConfiguration
When implementing it for go we also deprecated most of the old constructors
doing configuration without TConfiguration.
--
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]
