不能正确处理Protobuf的请求

[oliverzy]

你好，

如果Server启动Protobuf，并且定义了serverProtos.json，libPomelo2将无法解码。

跟了一下代码，发现pc_default_msg_decode中，msg.route没有办法获得，因为从Server回来的消息这个字段是没有的。

比对了JS的实现如下， if(msg.id > 0){ msg.route = routeMap[msg.id]; delete routeMap[msg.id]; if(!msg.route){ return; } } JS客户端回在请求发起前把request ID和route的关系存在一个map里面，回头收到响应的时候通过查这个表得到route，但是libpomelo2没有这个步骤，所以怀疑问题出在这儿，我自己看了一下，因为对代码不了解，还不知道该如何修改，请帮忙修复这个BUG，谢谢

[cynron]

关于这个问题，请参考讨论 http://nodejs.netease.com/topic/543aa8bccb93b0031f4f5f23

[Jennal]

@cynron 1月15日之前的版本还是可以的，刚更新完就不行了

[Jennal]

@cynron Sorry，我错了，我们虽然配了pb，但是没有配serverProtos，所以还是没有走pb的吧。

[cynron]

@Jennal @oliverzy

这个bug，目前不打算修了，以后可能会修改协议修复

[oliverzy]

@cynron @Jennal 我写了一个patch，请看看有没有问题，再官方修复之前，凑合着用一下

diff --git a/src/tr/uv/pr_msg.c b/src/tr/uv/pr_msg.c
index f002dd3..b15dddb 100644
--- a/src/tr/uv/pr_msg.c
+++ b/src/tr/uv/pr_msg.c
@@ -13,6 +13,8 @@
 #include "pr_msg.h"
 #include "tr_uv_tcp_i.h"

+#include "pc_pomelo_i.h"
+
 #define PC_MSG_FLAG_BYTES 1
 #define PC_MSG_ROUTE_LEN_BYTES 1
 #define PC_MSG_ROUTE_CODE_BYTES 2
@@ -152,7 +154,7 @@ error:
     return NULL;
 }

-pc_msg_t pc_default_msg_decode(const json_t* code2route, const json_t* server_protos, const pc_buf_t* buf) 
+pc_msg_t pc_default_msg_decode(const json_t* code2route, const json_t* server_protos, const pc_buf_t* buf, QUEUE* requestQueue)
 {
     const char *route_str = NULL;
     const char *origin_route = NULL;
@@ -199,7 +201,24 @@ pc_msg_t pc_default_msg_decode(const json_t* code2route, const json_t* server_pr

         msg.route = route_str;
     } else {
-        msg.route = NULL;
+        QUEUE* q;
+        pc_request_t* req;
+        QUEUE_FOREACH(q, requestQueue) {
+            req = (pc_request_t* )QUEUE_DATA(q, pc_common_req_t, queue);
+            if (req->req_id == msg.id) {
+                msg.route = (char* )pc_lib_malloc(strlen(req->base.route) + 1);
+                strcpy((char* )msg.route, req->base.route);
+
+                //msg.route = req->base.route;
+                break;
+            }
+        }
+        
+        if (msg.route == NULL) {
+            msg.id = PC_INVALID_REQ_ID;
+            pc_lib_free(raw_msg);
+            return msg;
+        }
     }

     if (PC_MSG_HAS_ROUTE(raw_msg->type) &&  !msg.route) {
@@ -471,6 +490,6 @@ pc_msg_t pr_default_msg_decoder(tr_uv_tcp_transport_t* tt, const uv_buf_t* buf)
     pc_buf_t pb;
     pb.base = buf->base;
     pb.len = buf->len;
-
-    return pc_default_msg_decode(tt->code_to_route, tt->server_protos, &pb);
+    
+    return pc_default_msg_decode(tt->code_to_route, tt->server_protos, &pb, &tt->client->req_queue);
 }
diff --git a/src/tr/uv/pr_msg.h b/src/tr/uv/pr_msg.h
index d383562..95d486a 100644
--- a/src/tr/uv/pr_msg.h
+++ b/src/tr/uv/pr_msg.h
@@ -8,6 +8,7 @@

 #include <jansson.h>
 #include <stdint.h>
+#include <queue.h>

 #include "pr_pkg.h"

@@ -31,7 +32,7 @@ typedef struct {
 } pc_buf_t;

 pc_buf_t pc_default_msg_encode(const json_t* route2code, const json_t* client_protos, const pc_msg_t* msg);
-pc_msg_t pc_default_msg_decode(const json_t* code2route, const json_t* server_protos, const pc_buf_t* buf);
+pc_msg_t pc_default_msg_decode(const json_t* code2route, const json_t* server_protos, const pc_buf_t* buf, QUEUE* requestQueue);

 pc_buf_t pc_body_json_encode(const json_t *msg);
 json_t *pc_body_json_decode(const char *data, size_t offset, size_t len);
diff --git a/src/tr/uv/tr_uv_tcp_aux.c b/src/tr/uv/tr_uv_tcp_aux.c
index 3c87b98..201f585 100644
--- a/src/tr/uv/tr_uv_tcp_aux.c
+++ b/src/tr/uv/tr_uv_tcp_aux.c
@@ -855,7 +855,7 @@ void tcp__on_data_recieved(tr_uv_tcp_transport_t* tt, const char* data, size_t l
     }

     assert((msg.id == PC_NOTIFY_PUSH_REQ_ID && msg.route)
-            || (msg.id != PC_NOTIFY_PUSH_REQ_ID && !msg.route));
+            || (msg.id != PC_NOTIFY_PUSH_REQ_ID && msg.route));

     pc_lib_log(PC_LOG_INFO, "tcp__on_data_recieved - recived data, req_id: %d", msg.id);
     if (msg.id != PC_NOTIFY_PUSH_REQ_ID) {

[cynron]

@oliverzy 你的这个patch，目测好像有问题，因为你在遍历request queue的时候，虽然是只读，但是没有锁保护，假如这个时候，其他线程又有request请求发出的话，会造成诡异问题.

其实你完全可以把pc_client_t作为参数传进去，这样就有锁可用了.

但是这样做，打破了层级依赖关系，造成了底层依赖高层的问题，这也是不打算修这个问题的原因

[Jennal]

@oliverzy 测试了你的方法，并在遍历request queue的时候加了锁，protobuf可以用了，非常感谢。

@cynron 不知道为什么要纠结于底层依赖高层，而不解决这个问题？对于游戏团队来说，如果游戏要上线，并且要用libpomelo2这个库的话，这个问题是必须解决的，无论怎么升级，这个问题都必须手工patch一次。

我认为在tt中保存一个pc_client_t*的指针，并且在tr_uv_tcp_init中调用了pc_client_config，已经存在所谓“底层依赖高层”的嫌疑了吧？这可能是在一开始没有把结构设计好的关系。但是影响功能的问题，应该要修复掉吧，就算代码结构可能会有问题，但是应该以功能优先去做这个开源库吧？大部分人并不会因为这个库的代码有多么优美而去用它，用它的唯一原因是好用。代码结构性的问题可以通过一版一版逐步迭代去解决，但是因为一个代码结构问题，而不去修BUG，这个太说不过去了吧。

一直说要解决要解决，通过修改协议来解决，会尽快解决，但是这个问题从去年10月份就有了，都快1年了，还没有解决。本来只是这么小的问题，何必这么纠结呢？

能不能先把这个问题修掉？即使代码结构让人不舒服，但是带着这个不舒服的代码，才更有动力去重构，不是吗？

[cynron]

@Jennal 抱歉，抱歉，一直没回应， ^ ^

你说的很有道理，解决问题最重要，哈哈.......

你可以把这个patch整理一下，发个pr吧，谢谢...

@oliverzy

[cynron]

我已经把wontfix标签去掉了

[felipejfc]

still no way for fixing serverProtos for server response on libpomelo2?