fluent/fluent-logger-java

org.fluentd.logger.sender.RawSocketSender#getBuffer should be thread safe or be private method

komamitsu opened this issue · 4 comments

I found org.fluentd.logger.sender.RawSocketSender#getBuffer changes the position of pendings buffer without any lock in spite of it's a public method. It means the user can call getBuffer() when calling flush() at the same time, and the sender instance can read a wrong position of the buffer.

Also, I don't know why org.fluentd.logger.sender.Sender has getBuffer(). I think the method shouldn't be exposed originally.

@muga What do you think? We can remove the method from org.fluentd.logger.sender.Sender and make it be a private method?

muga commented

@komamitsu

Basically I agree with your suggestion. The reason why I added getBuffer method in Sender interface is for unit testing. By call the method and checking the byte array in pendings, we can check the behavior of the sender easily. So, I recommend that the modifier of the method should be changed: public to protected (or package). how about it?

package scope sounds better. Also, we can call the method with casting an instance to RawSocketSender class even if removing the method from the interface.

muga commented

Good. Please do that:-)

Fixed > #28