protosubff : heartbeat data is null causes connect close

[chenyu2016]

What is the purpose of the change

repair protosubff heartbeat data is null causes connect close

Brief changelog

ExchangeCodec. decodeBody
     -->if (res.isHeartbeat()) {
                        // data = decodeHeartbeatData(channel, in);
                        data = null;
                    }
ExchangeCodec.encodeEventData(ObjectOutput out, Object data)
if(data != null) {
            out.writeObject(data);
        }else{
            logger.warn("encodeEventData  data is null");
        }

Verifying this change

Dubbo version: 2.7.1
Operating System version: mac
Java version: 1.8

[beiwei30]

@chenyu2016 pls. take a look at my comments.

[beiwei30]

ps, the following failure is found from travis CI 'HeartbeatHandlerTest.testHeartbeat:95 expected: <0> but was: <2>'. It will not happen if your change follows my suggestion.

[beiwei30]

@chenyu2016 CI still fails:

[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   ExchangeCodecTest.test_Decode_Error_Request_Object:296 expected: <true> but was: <false>
[ERROR]   ExchangeCodecTest.test_Decode_Return_Request_Event_Object:235 expected: <org.apache.dubbo.remoting.codec.TelnetCodecTest$Person@3c1> but was: <null>
[ERROR]   ExchangeCodecTest.test_Decode_Return_Request_Event_String:250 expected: <R> but was: <null>
[ERROR]   ExchangeCodecTest.test_Decode_Return_Request_Object:278 expected: <org.apache.dubbo.remoting.codec.TelnetCodecTest$Person@3c1> but was: <null>

[chenyu2016]

You're right.I have revised it. I think it's really redundant for me to do so.

…

------------------ 原始邮件 ------------------ 发件人: "Ian Luo"<[email protected]>; 发送时间: 2019年7月25日(星期四) 上午10:12 收件人: "apache/dubbo"<[email protected]>; 抄送: "疺喡亻苼"<[email protected]>;"Mention"<[email protected]>; 主题: Re: [apache/dubbo] protosubff : heartbeat data is null causes connectclose (#4645) @beiwei30 commented on this pull request. In dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/codec/ExchangeCodec.java: @@ -378,7 +379,11 @@ protected void encodeData(ObjectOutput out, Object data) throws IOException { } private void encodeEventData(ObjectOutput out, Object data) throws IOException { - out.writeObject(data); + if(data != null) { + out.writeObject(data); I think it's fine, but I suggest we still need to make encode heartbeat to no-op, what do you think: @deprecated protected void encodeHeartbeatData(ObjectOutput out, Object data) throws IOException { // no op since heartbeat_event is null } — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[beiwei30]

CI still fail:

[ERROR] Failures: 
[ERROR]   ExchangeCodecTest.test_Decode_Error_Request_Object:296 expected: <true> but was: <false>
[ERROR]   ExchangeCodecTest.test_Decode_Return_Request_Event_Object:235 expected: <org.apache.dubbo.remoting.codec.TelnetCodecTest$Person@3c1> but was: <null>
[ERROR]   ExchangeCodecTest.test_Decode_Return_Request_Event_String:250 expected: <R> but was: <null>
[ERROR]   ExchangeCodecTest.test_Decode_Return_Request_Object:278 expected: <org.apache.dubbo.remoting.codec.TelnetCodecTest$Person@3c1> but was: <null>
[INFO] 
[ERROR] Tests run: 342, Failures: 4, Errors: 0, Skipped: 1

[beiwei30]

@chenyu2016 I revisit this particular issue today, and the null heartbeat issue should have been solved by #3734.

But I also realize that encodeHeartbeatData and decodeHeartbeatData are in fact useless, therefore I submit a new pull request #4673 to address it.

Pls. verify if it works for you. I will close this pull request for now, feel free to reopen if you have different opinion.