toppers / hakoniwa-conductor

1 stars 0 forks source link

C関数にStringの文字列を渡す際に null終端させていない問題があった #12

Closed tmori closed 1 year ago

tmori commented 1 year ago

経緯:UDPスレッドが、以下のようにパニックした(色々とログを挟んだ結果です)

robo name_len=5
robo name=Drone
channel_id=0
pdu_size=432
get_pdu_channel args: channel_id=0 robo_namelen=5 robo_name=Drone
get_pdu_channel: 0: logical_channel_id=1 robo_namelen=5 robo_name=Drone
get_pdu_channel: 1: logical_channel_id=0 robo_namelen=5 robo_name=Drone
get_pdu_channel args: channel_id=0 robo_namelen=6 robo_name=Drone
get_pdu_channel: 0: logical_channel_id=1 robo_namelen=5 robo_name=Drone
get_pdu_channel: 1: logical_channel_id=0 robo_namelen=5 robo_name=Drone
get_pdu_channel: 2: logical_channel_id=2 robo_namelen=5 robo_name=Drone
get_pdu_channel: 3: logical_channel_id=3 robo_namelen=5 robo_name=Drone
write_pdu real_id is not found: channel_id=0
ERROR: write_pdu: len=432 > channel[0].size(48)
thread '<unnamed>' panicked at 'assertion failed: ret == true', src/hako/method/udp/mod.rs:62:21
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

原因: C++箱庭コア機能(get_pdu_channel())で、robo_name(Drone) の文字列長をチェックしたら、サイズが1個多いため、対象の名前検索で失敗した。 サイズが1個多かった原因のデータは以下の通り。

0 : 0x44 1 : 0x72 2 : 0x6f 3 : 0x6e 4 : 0x65 5 : 0x7f

最後に制御コードが入ってしまっている。

処理の流れは以下の通り。

  1. rust: UnityからUDPデータ受信
  2. UDPデータをデコード
  3. robo_nameを取得した。この時点での文字列長は5(正解)
  4. write_asset_pub_pdu()に robo_nameを渡した
  5. asset_get_pdu_channel()呼び出しでは、文字列長5としてC++版箱庭コア機能を呼び出しており問題なし。
  6. 次に、pi::asset_write_pdu()で robo_nameを c_charに変換してC++版箱庭コア機能を呼び出すと、C++側では、文字列長を6と認識してしまっていた。

そのため、以下の箇所に、null終端コードを入れればよいと考えたが、この処理を実行すると、上記の5 でパニックしてしまった。rustのデータとしては、null終端文字コードはない方が良い??

https://github.com/toppers/hakoniwa-conductor/blob/30cfcc217d59e39b862b6592465330d073df44c7/main/src/hako/method/udp/mod.rs#L49-L54

対応案としては、asset_get_pdu_channel()の内部実装を真似することが近道と思われる。

https://github.com/toppers/hakoniwa-conductor/blob/30cfcc217d59e39b862b6592465330d073df44c7/main/src/hako/api/mod.rs#L212-L214

tmori commented 1 year ago

上記修正で問題はでなくなったけれど、問題の本質的な解決になっていないことに気づきました。

該当コードはここ。 https://github.com/toppers/hakoniwa-conductor/blob/30cfcc217d59e39b862b6592465330d073df44c7/main/src/hako/pdu/mod.rs#L146

pdu.robo_name.as_ptr() のデータがおかしいの原因なので、pduの登録処理を直すべきと思えてきました。

tmori commented 1 year ago

この修正を適用してみましたが、同じ結果(パニック)でした。

https://github.com/toppers/hakoniwa-conductor/commit/419f0478c805a2ed2a4ae76f0b6a4da3bd7f64e1

ASSET_DEF=asset_def.txt
INFO: ACTIVATING MOSQUITTO
[49927.744212]~DLT~   16~INFO     ~FIFO /tmp/dlt cannot be opened. Retrying later...
INFO: ACTIVATING HAKO-MASTER
OPEN RECIEVER UDP PORT=172.25.195.216:54001
OPEN SENDER UDP PORT=172.25.195.216:54002
mqtt_url=mqtt://172.25.195.216:1883
PUBLISHER Connecting to the MQTT server...
PUBLISHER CONNECTED to the MQTT server...
delta_msec = 20
max_delay_msec = 100
INFO: shmget() key=255 size=80768 
Server Start: 172.25.195.216:50051
INFO: ACTIVATING :dev/ai/sample_drone.py
START DRONE TEST
LOADED: Drone
INFO: Drone create_lchannel: logical_id=1 real_id=0 size=48
subscribe:channel_id=0
subscribe:typename=Imu
subscribe:pdu_size=432
subscribe:channel_id=2
subscribe:typename=LaserScan
subscribe:pdu_size=3044
subscribe:channel_id=3
subscribe:typename=CompressedImage
subscribe:pdu_size=1229064
WAIT START:
subscribe_pdu_channel: Got a request: Request { metadata: MetadataMap { headers: {"te": "trailers", "content-type": "application/grpc", "user-agent": "grpc-csharp/2.37.0-dev grpc-c/15.0.0 (windows; chttp2)", "grpc-accept-encoding": "identity,deflate,gzip", "accept-encoding": "identity,gzip"} }, message: SubscribePduChannelRequest { asset_name: "UnityAsset", channel_id: 1, pdu_size: 48, listen_udp_ip_port: "172.25.192.1:54003", method_type: "UDP", robo_name: "Drone" }, extensions: Extensions }
create_asset_sub_pdu
create_pdu_channel: Got a request: Request { metadata: MetadataMap { headers: {"te": "trailers", "content-type": "application/grpc", "user-agent": "grpc-csharp/2.37.0-dev grpc-c/15.0.0 (windows; chttp2)", "grpc-accept-encoding": "identity,deflate,gzip", "accept-encoding": "identity,gzip"} }, message: CreatePduChannelRequest { asset_name: "UnityAsset", channel_id: 0, pdu_size: 432, method_type: "UDP", robo_name: "Drone" }, extensions: Extensions }
INFO: Drone create_lchannel: logical_id=0 real_id=1 size=432
create_asset_pub_pdu
create_asset_pub_pdu: channel_ID=0
create_pdu_channel: Got a request: Request { metadata: MetadataMap { headers: {"te": "trailers", "content-type": "application/grpc", "user-agent": "grpc-csharp/2.37.0-dev grpc-c/15.0.0 (windows; chttp2)", "grpc-accept-encoding": "identity,deflate,gzip", "accept-encoding": "identity,gzip"} }, message: CreatePduChannelRequest { asset_name: "UnityAsset", channel_id: 2, pdu_size: 3044, method_type: "MQTT", robo_name: "Drone" }, extensions: Extensions }
INFO: Drone create_lchannel: logical_id=2 real_id=2 size=3044
create_asset_pub_pdu
create_asset_pub_pdu: channel_ID=2
create_pdu_channel: Got a request: Request { metadata: MetadataMap { headers: {"te": "trailers", "content-type": "application/grpc", "user-agent": "grpc-csharp/2.37.0-dev grpc-c/15.0.0 (windows; chttp2)", "grpc-accept-encoding": "identity,deflate,gzip", "accept-encoding": "identity,gzip"} }, message: CreatePduChannelRequest { asset_name: "UnityAsset", channel_id: 3, pdu_size: 1229064, method_type: "MQTT", robo_name: "Drone" }, extensions: Extensions }
INFO: Drone create_lchannel: logical_id=3 real_id=3 size=1229064
create_asset_pub_pdu
create_asset_pub_pdu: channel_ID=3
register: Got a request: Request { metadata: MetadataMap { headers: {"te": "trailers", "content-type": "application/grpc", "user-agent": "grpc-csharp/2.37.0-dev grpc-c/15.0.0 (windows; chttp2)", "grpc-accept-encoding": "identity,deflate,gzip", "accept-encoding": "identity,gzip"} }, message: AssetInfo { name: "UnityAsset" }, extensions: Extensions }
asset_notification_start: Got a request: Request { metadata: MetadataMap { headers: {"te": "trailers", "content-type": "application/grpc", "user-agent": "grpc-csharp/2.37.0-dev grpc-c/15.0.0 (windows; chttp2)", "grpc-accept-encoding": "identity,deflate,gzip", "accept-encoding": "identity,gzip"} }, message: AssetInfo { name: "UnityAsset" }, extensions: Extensions }
start_simulation: Got a request: Request { metadata: MetadataMap { headers: {"te": "trailers", "content-type": "application/grpc", "user-agent": "grpc-csharp/2.37.0-dev grpc-c/15.0.0 (windows; chttp2)", "grpc-accept-encoding": "identity,deflate,gzip", "accept-encoding": "identity,gzip"} }, message: (), extensions: Extensions }
WAIT RUNNING:
create topic: real_id=2 method_type=MQTT
create topic: hako_mqtt_Drone_2
create topic: real_id=1 method_type=UDP
create topic: real_id=3 method_type=MQTT
create topic: hako_mqtt_Drone_3
## SimulationAssetEvent START
asset_notification_feedback: Got a request: Request { metadata: MetadataMap { headers: {"te": "trailers", "content-type": "application/grpc", "user-agent": "grpc-csharp/2.37.0-dev grpc-c/15.0.0 (windows; chttp2)", "grpc-accept-encoding": "identity,deflate,gzip", "accept-encoding": "identity,gzip"} }, message: AssetNotificationReply { event: Start, asset: Some(AssetInfo { name: "UnityAsset" }), ercd: Ok }, extensions: Extensions }
START CREATE PDU DATA: total_size= 1232588
INFO: shmget() key=256 size=1232588 
PDU DATA CREATED
CREATED ADDR=0x7f277e2b700c
GO:
LOADED: PDU DATA
sync_mode: true
simulation mode: false
thread '<unnamed>' panicked at 'assertion failed: ret == true', src/hako/method/udp/mod.rs:60:21
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
tmori commented 1 year ago

以下の修正を適用したら動きました。

    api::asset_write_pdu(
        pdu.asset_name.as_ptr() as *const c_char,
        //pdu.robo_name.as_ptr() as *const c_char, 
        robo_name.clone().as_ptr() as *const c_char,
tmori commented 1 year ago

以下の修正でも動きました。

api::asset_write_pdu(
    pdu.asset_name.as_ptr() as *const c_char,
    //pdu.robo_name.as_ptr() as *const c_char, 
    pdu.robo_name.clone().as_ptr() as *const c_char, 
tmori commented 1 year ago

上記から推測すると、C++版箱庭コア機能にポインタを渡す場合は、clone()しておく必要がありそうですね。

tmori commented 1 year ago

全体的に直しました。

tmori commented 1 year ago

この修正で、問題も再発しなくなりました。

tmori commented 1 year ago

以下のPRで問題解決しました。 結局、CString でポインタを作らないとダメのようです。

https://github.com/toppers/hakoniwa-conductor/pulls