chenggwang commented on PR #671:
URL: https://github.com/apache/tomcat/pull/671#issuecomment-1762749332
> The benefit seems null IMO, in the debugger it will print out that this is
null instead of a NPE, which should mean the same for a developer. Also,
Nio2Channel.toString is the same.
Following is the`NioChannel` attribute `SocketChannel sc`, along with its
`constructor`, `rest` methods and `toString` method.We initialize a
`NioChannel` through the constructor, only the `bufHandler` property is
assigned a value, and its `sc` property remains null. At this point the object
is nearly created, meaning we can use its properties and methods as we wish.
But when we use its toString() method, `super.toString() + ":" +
sc.toString()`, `sc` is null, so it throws an NPE. The `sc` is not assigned a
value until we call the reset method. Otherwise, sc.toString() throws NPE
directly instead of printing null
```java
public class NioChannel implements ByteChannel, ScatteringByteChannel,
GatheringByteChannel {
... Omit extraneous code
protected final SocketBufferHandler bufHandler;
protected SocketChannel sc = null;
protected NioSocketWrapper socketWrapper = null;
public NioChannel(SocketBufferHandler bufHandler) {
this.bufHandler = bufHandler;
}
...
public String toString() {
return super.toString() + ":" + sc.toString();
}
public void reset(SocketChannel channel, NioSocketWrapper socketWrapper)
throws IOException {
this.sc = channel;
this.socketWrapper = socketWrapper;
bufHandler.reset();
}
...Omit extraneous code
}
```
Going back to the object-oriented level, when we provide an object, we need
to make sure that the basic functionality it provides to the outside world
after initialization is complete is usable and reliable. The basic
functionality of course includes the `toString()` method, because this is its
calling card, through which the outside world should know the basic information
about the object. This is also the original purpose of the toString method.
On line 12 we create a channel with `createChannel(bufhandler)`，At this
point we are calling the constructor of the NioChannel.This is the same
explanation as I started with, sc is still null and all calls to channel's`
toString` method before line 14 report NPE.
Similarly `NioSocketWrapper newWrapper`, the `newWrapper.toString()` method
throws an NPE until the rest method is called, as it looks like this
`super.toString() + ":" + String.valueOf(socket)`, ` String.valueOf(socket)`
still calls socket(NioChannel)'s `sc.toString`, when `sc` is null.
```java
line 1 protected boolean setSocketOptions(SocketChannel socket) {
line 2 NioSocketWrapper socketWrapper = null;
line 3 try {
// Allocate channel and wrapper
line 4 NioChannel channel = null;
line 5 if (nioChannels != null) {
line 6 channel = nioChannels.pop();
}
line 7 if (channel == null) {
line 8 SocketBufferHandler bufhandler = new
SocketBufferHandler(
line 9 socketProperties.getAppReadBufSize(),
line 10 socketProperties.getAppWriteBufSize(),
line 11 socketProperties.getDirectBuffer());
line 12 channel = createChannel(bufhandler);
}
line 13 NioSocketWrapper newWrapper = new
NioSocketWrapper(channel, this);
line 14 channel.reset(socket, newWrapper);
line 15 connections.put(socket, newWrapper);
line 16 socketWrapper = newWrapper;
```
createChannel method
```java
protected NioChannel createChannel(SocketBufferHandler buffer) {
if (isSSLEnabled()) {
return new SecureNioChannel(buffer, this);
}
return new NioChannel(buffer);
}
public NioChannel(SocketBufferHandler bufHandler) {
this.bufHandler = bufHandler;
}
```
<img width="709" alt="屏幕截图 2023-10-14 163405"
src="https://github.com/apache/tomcat/assets/90715678/44871f56-e5a2-435e-86cf-e15063806971";>
Above is the `System.out.println` I added between line 13 and 14 to output
the `channel` and `newWrapper` objects, throwing NPE's.I have a monitoring
project that tracks changes in the life cycle of an object after any object is
created, we are monitoring all constructors, reflections, and start logging as
soon as an object is created. The two lines `System.out.println` in the diagram
are similar to the abstraction of our monitoring function as a buried point. To
clarify our buried points are at the bytecode level, there is no hardcoding of
tomcat source code dumping. However, the NPE occurred when logging, so I went
down to the source code to debug it, and found that it was the reason that
toString() didn't judge the null.
The `toString` method doesn't seem to have much use or impact on the
application itself, but it's still useful for monitoring and logging. So if
tomcat side does not change, I can only in my monitoring project to make a
patch to find NioChannel, wait for the rest method to execute and then track
the channel object, haha but I feel that my code is very ugly!
Still hope that the official side of the adjustment, I submit also PR also
found a lot of toString method with "+" splicing, high version of the jdk
compilation will automatically StringBuilder, so did not change the + sign.
Here judgment empty for tomcat performance is almost no consumption!
--
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: dev-unsubscr...@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
