flutter-tizen / plugins

Flutter plugins for Tizen
66 stars 47 forks source link

[tizen_rpc_port] Cannot reconnect after hot restart #504

Closed swift-kim closed 11 months ago

swift-kim commented 1 year ago

How to reproduce:

  1. Launch the server example app.
  2. Launch the client example app and connect to the server.
  3. Hot restart the server app. The client is not automatically disconnected.
  4. Send a message from the client to the server. This fails with "Input/output error".
  5. Try to reconnect to the server. This fails with a "connect failed" error.

The plugin has so many other bugs I found during testing and I can't even list them all here. The plugin is currently NOT USABLE at all and will need way more improvements until it becomes stable enough for production use.

swift-kim commented 1 year ago

Another issue: It seems the plugin (or the underlying native library) blocks the main thread (platform thread) for more than a few seconds (I tested on a mobile emulator) while it's transferring data. Is it a joke or something?

Thread 1 "Runner.dll" received signal SIGINT, Interrupt.
0xb7781a51 in __kernel_vsyscall ()
(gdb) bt
#0  0xb7781a51 in __kernel_vsyscall ()
#1  0xb73b415b in __GI___poll (timeout=1000, nfds=1, fds=0xbfff203c) at ../sysdeps/unix/sysv/linux/poll.c:29
#2  __GI___poll (fds=0xbfff203c, nfds=1, timeout=1000) at ../sysdeps/unix/sysv/linux/poll.c:26
#3  0xa3392975 in rpc_port::internal::Port::CanRead(int) () from target:/lib/librpc-port.so.1
#4  0xa3392c7e in rpc_port::internal::Port::Read(void*, unsigned int) () from target:/lib/librpc-port.so.1
#5  0xa339c328 in rpc_port_read () from target:/lib/librpc-port.so.1
#6  0xa3399ff8 in rpc_port_parcel_create_from_port () from target:/lib/librpc-port.so.1
#7  0x9821bc83 in tizen::RpcPortProxyManager::OnReceivedEvent (receiver=0x818a4550 "org.tizen.tizen_rpc_port_server_example", port_name=0xb480e1f4 "Message", data=0xb483ba50)
    at src/rpc_port_proxy.cc:142
#8  0xa339bc1b in ?? () from target:/lib/librpc-port.so.1
#9  0xa3395b72 in rpc_port::internal::Proxy::ProxyPort::OnDataReceived(_GIOChannel*, GIOCondition, void*) () from target:/lib/librpc-port.so.1
#10 0xb6f326b3 in g_main_context_dispatch () from target:/lib/libglib-2.0.so.0
#11 0xb6c1b566 in ?? () from target:/lib/libecore.so.1
#12 0xb6c1e0ba in ?? () from target:/lib/libecore.so.1
#13 0xb6c1e8e3 in ?? () from target:/lib/libecore.so.1
#14 0xb6c1f09b in ?? () from target:/lib/libecore.so.1
#15 0xb6c24476 in ?? () from target:/lib/libecore.so.1
#16 0xb6c23374 in efl_loop_begin () from target:/lib/libecore.so.1
#17 0xb6c1d458 in ecore_main_loop_begin () from target:/lib/libecore.so.1
swift-kim commented 1 year ago

https://github.com/flutter-tizen/plugins/pull/505 를 통해서 몇 가지 부분을 정리해봤지만 여전히 다음과 같은 심각한 문제들이 남아있습니다.

  1. Tizen Studio 5.0의 최신 TIDLC 1.10.6이 생성하는 코드와 현재 example 앱들에 포함된 코드가 서로 다릅니다.

    저번에 이 패키지를 추가하기 전에 generate된 코드를 리뷰하고 변경 사항들을 적용했습니다만 TIDLC에는 전혀 반영이 안 된 것 같습니다.

  2. 현재 client app의 _client.send()가 signature 상으로는 async지만 실제로는 blocking call입니다.

    때문에 server 측에서 처리가 오래 걸릴 경우 그동안 client의 UI thread가 멈추게 되는 (jank) 현상이 발생합니다. TIDL 인터페이스에 async로 명시해도 딱히 generate된 코드에는 차이가 없고 의미가 없어보입니다.

  3. client가 접속 중인 상태에서 server를 재시작하면 여러 예기치 못한 문제가 발생합니다. (https://github.com/flutter-tizen/plugins/pull/505 에서 수정된 코드를 기준으로 7.0 mobile 에뮬레이터에서 테스트했습니다.)

    (1) 예를 들어 제목에서 얘기한 것처럼 server를 hot restart (Shift+R) 할 경우 client는 아직 접속 중인 상태로 인식하며 (자동으로 disconnect되지 않음) 이 상태에서 send를 실행하면 몇 초간 UI가 멈춘 다음 "Input/output error" 로그가 출력되고 아무 일도 일어나지 않습니다. (해당 로그는 _consumeCommandcatch clause에서 발생)

    (2) server 앱을 종료시키고 다시 실행할 경우 client는 접속이 끊어지고 다시 접속이 가능하지만, send 실행 시 몇 초간 다시 UI가 멈추고 "Input/output error"로 인한 PlatformException이 발생합니다. (해당 에러는 RpcPortProxyManagerrpc_port_parcel_create_from_port에서 발생) 단, callback은 정상적으로 실행되는 것으로 파악됩니다.

  4. 저번 리뷰에서 언급한 것처럼 server API 중에 listen을 cancel할 수 있는 수단이 없습니다. 이는 3-(1)의 문제의 원인이 됩니다.

    또한 현재 client example 앱의 dispose()에서 _client.disconnect()를 부르지 않고 있는데 이는 의도된 것인지 궁금합니다.

@upple @hjhun PR에 대한 리뷰도 부탁드립니다.

hjhun commented 1 year ago

505 를 통해서 몇 가지 부분을 정리해봤지만 여전히 다음과 같은 심각한 문제들이 남아있습니다.

  1. Tizen Studio 5.0의 최신 TIDLC 1.10.6이 생성하는 코드와 현재 example 앱들에 포함된 코드가 서로 다릅니다. 저번에 이 패키지를 추가하기 전에 generate된 코드를 리뷰하고 변경 사항들을 적용했습니다만 TIDLC에는 전혀 반영이 안 된 것 같습니다.

    @upple 님 이 부분 확인 부탁드립니다. 리뷰과정에서 적용했던 내용들이 tidlc에 적용 안된 부분이 있으면 수정 부탁드리겠습니다.

  2. 현재 client app의 _client.send()가 signature 상으로는 async지만 실제로는 blocking call입니다. 때문에 server 측에서 처리가 오래 걸릴 경우 그동안 client의 UI thread가 멈추게 되는 (jank) 현상이 발생합니다. TIDL 인터페이스에 async로 명시해도 딱히 generate된 코드에는 차이가 없고 의미가 없어보입니다.

    TIDL interface에 명시한 send()는 sync입니다. sync로 동작해야되는게 맞습니다. async keyword를 제거하는 방향이 맞을 것 같습니다. 이 부분은 @upple 님과 논의해서 수정패치를 준비해보겠습니다.

  3. client가 접속 중인 상태에서 server를 재시작하면 여러 예기치 못한 문제가 발생합니다. ([tizen_rpc_port] Various cleanups #505 에서 수정된 코드를 기준으로 7.0 mobile 에뮬레이터에서 테스트했습니다.) (1) 예를 들어 제목에서 얘기한 것처럼 server를 hot restart (Shift+R) 할 경우 client는 아직 접속 중인 상태로 인식하며 (자동으로 disconnect되지 않음) 이 상태에서 send를 실행하면 몇 초간 UI가 멈춘 다음 "Input/output error" 로그가 출력되고 아무 일도 일어나지 않습니다. (해당 로그는 _consumeCommandcatch clause에서 발생)

    hot restart에 대한 동작은 확인을 해보고 말씀드리겠습니다. socket fd가 끊어지지 않는다면 client 쪽에 에러가 전달되지 않는 구조입니다.

    (2) server 앱을 종료시키고 다시 실행할 경우 client는 접속이 끊어지고 다시 접속이 가능하지만, send 실행 시 몇 초간 다시 UI가 멈추고 "Input/output error"로 인한 PlatformException이 발생합니다. (해당 에러는 RpcPortProxyManagerrpc_port_parcel_create_from_port에서 발생) 단, callback은 정상적으로 실행되는 것으로 파악됩니다.

    server에서 보낸 delegate invoke를 처리하는 과정에서 발생한 exception으로 보입니다. 이전 connection으로 인해 발생한것인지는 정확하지 않아 재현을 통해서 확인을 해봐야 정확하게 알 수 있을 것 같습니다.

  4. 저번 리뷰에서 언급한 것처럼 server API 중에 listen을 cancel할 수 있는 수단이 없습니다. 이는 3-(1)의 문제의 원인이 됩니다. 또한 현재 client example 앱의 dispose()에서 _client.disconnect()를 부르지 않고 있는데 이는 의도된 것인지 궁금합니다.

최초 디자인상에 server / client instance(handle)를 제거하게 되면 연결을 해제시키는게 의도된 구현이였습니다. disconnect()는 이후에 강제로 연결을 해제시키고 싶을때 API가 필요하다는 요청이 와서 추가된 API 입니다. dispose()가 호출되면 client의 handle이 제거되는데 이때 연결된 connection(fd)가 제거되면서 연결이 해제됩니다.

@upple @hjhun PR에 대한 리뷰도 부탁드립니다.

패치 준비 감사드립니다. @upple 님과 같이 리뷰진행하도록 하겠습니다.

swift-kim commented 1 year ago

TIDL interface에 명시한 send()는 sync입니다. sync로 동작해야되는게 맞습니다. async keyword를 제거하는 방향이 맞을 것 같습니다. 이 부분은 @upple 님과 논의해서 수정패치를 준비해보겠습니다.

예, 인터페이스에서 async로 지정하는 경우도 고려해주시면 좋겠습니다. 현재는 메서드 signature만 async이고 내부 구현은 sync여서 call을 하면 await를 붙여도 blocking이 발생합니다.

hot restart에 대한 동작은 확인을 해보고 말씀드리겠습니다. socket fd가 끊어지지 않는다면 client 쪽에 에러가 전달되지 않는 구조입니다.

4에서 말씀드린 것처럼 정상적인 구조라면 앱이 재시작되기 전에 dispose()에서 리소스를 해제하는 과정이 (즉 listen을 cancel하는) 있어야 하는데 API 구현이 안 되어 있습니다.

dispose()가 호출되면 client의 handle이 제거되는데 이때 연결된 connection(fd)가 제거되면서 연결이 해제됩니다.

dispose시에 딱히 그러한 동작이 발생하지는 않습니다. GC finalizer는 특정 시점이 아닌 임의의 시점에 Dart VM에 의해 실행됩니다 (C#과 유사). 현재 example처럼 간단한 구조의 앱에서는 문제가 없을 수도 있으나 올바른 GC의 활용법은 아니라고 할 수 있습니다. 위에 언급한 것과 같은 맥락으로 client도 server와 마찬가지로 dispose()에서 명시적으로 리소스를 해제하도록 (disconnect) 하는 것이 적절한 디자인으로 보입니다.

swift-kim commented 1 year ago

https://github.com/flutter-tizen/plugins/issues/504#issuecomment-1362474968

이 문제에 대해서도 고려를 부탁드립니다. 모듈 내부적으로 polling이 발생하는 것 같고 UI jank를 피하려면 플러그인 구현에 threading을 추가하는 방법이 있기는 하지만... rpc_port_parcel_create_from_port 자체가 async가 되는 게 더 적절해 보이긴 합니다. 그런데 API가 추가되어도 Tizen 7.0에나 들어올 텐데 plugin의 target API version을 7.0으로 올리는 것도 바람직하지는 않고.. 더 적절한 해결책이 있을지는 모르겠네요.

hjhun commented 1 year ago
12-27 10:46:53.576+0900 D/RPC_PORT (P 9775, T 9800): stub-internal.cc: Stub(91) > Stub::Stub()

12-27 10:46:53.576+0900 W/RPC_PORT (P 9775, T 9800): rpc-port.cc: rpc_port_stub_create(432) > rpc_port_stub_create(0xb182bca0, Message)

12-27 10:46:53.587+0900 D/TizenRpcPortPlugin (P 9775, T 9775): tizen_rpc_port_plugin.cc: HandleMethodCall(103) > HandleMethodCall. method_name: stubListen

12-27 10:46:53.587+0900 E/TizenRpcPortPlugin (P 9775, T 9775): tizen_rpc_port_plugin.cc: StubListen(177) > ptr: 0xb182bca0

12-27 10:46:53.587+0900 D/TizenRpcPortPlugin (P 9775, T 9775): rpc_port_stub.cc: Listen(38) > Listen: 0xb182bca0

12-27 10:46:53.587+0900 W/RPC_PORT (P 9775, T 9775): rpc-port.cc: rpc_port_stub_listen(470) > rpc_port_stub_listen(0xb182bca0)

12-27 10:46:53.588+0900 W/AUL      (P 9775, T 9775): app_request.cc: SendSimply(125) > Request cmd(165:RPC_PORT_CREATE): target_uid(5001)

12-27 10:46:53.588+0900 D/AUL      (P 9775, T 9775): aul_sock.cc: SendAndReceive(223) > fd(135), cmd(165)

12-27 10:46:53.588+0900 W/AUL      (P 9775, T 9775): app_request.cc: SendSimply(131) > Request cmd(165:RPC_PORT_CREATE): result(135)

12-27 10:46:53.588+0900 W/AMD      (P 2241, T 2241): request_manager.cc: RequestHandler(493) > cyanra check. cmd(RPC_PORT_CREATE:165), caller_pid(9775), req_id(2010)

12-27 10:46:53.588+0900 I/AMD      (P 2241, T 2241): request_manager.cc: DispatchRequest(306) > cmd(RPC_PORT_CREATE:165), req_id(2010), caller_pid(9775), caller_uid(5001), clifd(37)

12-27 10:46:53.588+0900 W/AMD_RPC_PORT (P 2241, T 2241): port_manager.cc: RemovePort(93) > port_name(Message), pid(9775)

12-27 10:46:53.588+0900 I/AMD_RPC_PORT (P 2241, T 2241): amd_rpc_port.cc: DispatchRpcPortCreate(356) > [__RPC_PORT__] port_name(Message), pid(9775), fd(43)

12-27 10:46:54.142+0900 D/TizenRpcPortPlugin (P 9775, T 9775): tizen_rpc_port_plugin.cc: operator()(201) > OnCancel stub event channel

12-27 10:46:54.142+0900 D/TizenRpcPortPlugin (P 9775, T 9775): tizen_rpc_port_plugin.cc: operator()(189) > OnListen stub event channel:

12-27 10:46:54.143+0900 D/TizenRpcPortPlugin (P 9775, T 9775): rpc_port_stub.cc: Listen(38) > Listen: 0x91c21280

12-27 10:46:54.143+0900 W/RPC_PORT (P 9775, T 9775): rpc-port.cc: rpc_port_stub_listen(470) > rpc_port_stub_listen(0x91c21280)

Hot restart 시 이전 구현에 대한게 정리되어야 하는데 그렇지 못한것 같습니다. 로그를 보면 다시 시작된 Dart 앱이 listen을 호출해서 새로 커넥션을 맺지만 기존 연결은 그대로 남아있습니다. 그리고 OnCancel()이 불리고 다시 OnListen()이 시작되면서 기존 커넥션을 다시 사용하려는 동작이 나타납니다.

Hot restart 시에 이전에 동작하는 앱 코드가 제대로 정리되지 않는 상황으로 인지를 해야되는걸까요?

swift-kim commented 1 year ago

@hjhun 네, hot restart는 프로세스 전체가 종료되고 다시 시작하는 게 아니라 Dart VM 단에서 발생하는 동작이므로 native 리소스가 자동으로 정리되지 않습니다. 이 문제의 핵심은 hot restart가 아니라 hot restart가 아니더라도 사용자의 앱 구조에 따라 listen과 cancel이 반복될 수 있어야 (일반적으로 initStatedispose에서) 하는데 그러한 수단이 없어서 발생합니다.

hjhun commented 1 year ago

#504 (comment)

이 문제에 대해서도 고려를 부탁드립니다. 모듈 내부적으로 polling이 발생하는 것 같고 UI jank를 피하려면 플러그인 구현에 threading을 추가하는 방법이 있기는 하지만... rpc_port_parcel_create_from_port 자체가 async가 되는 게 더 적절해 보이긴 합니다. 그런데 API가 추가되어도 Tizen 7.0에나 들어올 텐데 plugin의 target API version을 7.0으로 올리는 것도 바람직하지는 않고.. 더 적절한 해결책이 있을지는 모르겠네요.

이 문제는 server <-> client가 연결을 맺은 상태에서 server가 hot restart로 재실행이 되었고 client 그 상태에서 message send()를 할때 나타나는 현상일까요? 재현을 해보면 비슷한 상황이 재현되는데 EOF error가 발생합니다. OnDataReceived()에서 rpc_port_parcel_create_from_port()의 호출은 IO event로 event가 전달되었을때 이를 처리하려고 호출되는 코드입니다. 내부적으로 polling이 있는건 데이터가 모두 오지 않았을 경우를 대비한 경우입니다.

제가 말씀드린 상황이 재현 시나리오가 맞다면 stub connection이 제대로 정리되지 않은 상황이라서 발생하는 문제인것으로 보입니다.

swift-kim commented 1 year ago

이 문제는 server <-> client가 연결을 맺은 상태에서 server가 hot restart로 재실행이 되었고 client 그 상태에서 message send()를 할때 나타나는 현상일까요?

@hjhun 네, 위에서 말씀드린 3-(1), 3-(2) 경우에 모두 발생하는 것으로 기억합니다. 비정상적인 상황으로 인해 예기치 못한 시간이 소요된다는 말씀이신데 정상적인 경우에는 해당 부분이 1프레임 시간 (16ms) 이상 blocking될 염려가 없다고 봐도 될까요?

hjhun commented 1 year ago

@hjhun 네, hot restart는 프로세스 전체가 종료되고 다시 시작하는 게 아니라 Dart VM 단에서 발생하는 동작이므로 native 리소스가 자동으로 정리되지 않습니다. 이 문제의 핵심은 hot restart가 아니라 hot restart가 아니더라도 사용자의 앱 구조에 따라 listen과 cancel이 반복될 수 있어야 (일반적으로 initStatedispose에서) 하는데 그러한 수단이 없어서 발생합니다.

기존 Dart 앱이 사용되지 않게되니 기존 앱의 리소스가 정리되면서 Dart VM에서 기존 리소스를 정리시킬것으로 예상했는데 그렇지 않은 것 같네요;

상황이 이렇다면 말씀하신대로 cancel 시킬 수 있는 API가 있어야 될 것 같습니다. 관련해서 고민을 좀 해보고 다시 코멘트를 남기도록 하겠습니다.

hjhun commented 1 year ago

이 문제는 server <-> client가 연결을 맺은 상태에서 server가 hot restart로 재실행이 되었고 client 그 상태에서 message send()를 할때 나타나는 현상일까요?

@hjhun 네, 위에서 말씀드린 3-(1), 3-(2) 경우에 모두 발생하는 것으로 기억합니다. 비정상적인 상황으로 인해 예기치 못한 시간이 소요된다는 말씀이신데 정상적인 경우에는 해당 부분이 1프레임 시간 (16ms) 이상 blocking될 염려가 없다고 봐도 될까요?

보통의 정상적인 경우에는 큰 문제 없이 통신되었던걸로 기억하고 있습니다. 사실 이 부분에 대해서 앱 개발자로부터 언급된 적이 없어서 큰 문제로 인지되지 않았던 것 같습니다.

코드 동작을 보면 문제가 있는 경우에는 최악의 경우에 10초까지도 블럭될 수 있는 동작이라 receiving thread 도입을 좀 고민해봐야겠습니다. 이 내용은 native에도 해당하는 내용이라 native rpc-port lib 내부를 수정해서 해결해야 될 것 같습니다. 현재 일정상 이 부분에 대한 수정은 당장하기 어려울 것 같습니다. 시간이 될때 패치를 준비해서 검토를 좀 진행해보고 적용될때 별도로 노티를 드리겠습니다.

swift-kim commented 1 year ago

@hjhun 네 답변 감사합니다. 구현 내용은 잘 모르지만 비정상 상황일 때 에러를 리턴하는 것도 방법일 것 같습니다.

hjhun commented 1 year ago

@hjhun 네, hot restart는 프로세스 전체가 종료되고 다시 시작하는 게 아니라 Dart VM 단에서 발생하는 동작이므로 native 리소스가 자동으로 정리되지 않습니다. 이 문제의 핵심은 hot restart가 아니라 hot restart가 아니더라도 사용자의 앱 구조에 따라 listen과 cancel이 반복될 수 있어야 (일반적으로 initStatedispose에서) 하는데 그러한 수단이 없어서 발생합니다.

기존 Dart 앱이 사용되지 않게되니 기존 앱의 리소스가 정리되면서 Dart VM에서 기존 리소스를 정리시킬것으로 예상했는데 그렇지 않은 것 같네요;

상황이 이렇다면 말씀하신대로 cancel 시킬 수 있는 API가 있어야 될 것 같습니다. 관련해서 고민을 좀 해보고 다시 코멘트를 남기도록 하겠습니다.

https://docs.flutter.dev/development/tools/hot-reload hot restart 설명을 읽어보면 앱 상태를 잃어버린다고 되어있는데 Dart VM이 앱을 재시작 시켜도 기존 앱 리소스가 끝까지 정리 안되는 것 같습니다. 이 부분은 Dart VM의 버그일까요? 동작이 기대한 것(이후에 리소스 정리)과 달라서 한번 언급드려 봅니다.

client(proxy) API의 경우 cancel()의 추가는 어려운 상황은 아닌 것 같습니다. 내부적으로 disconnect() API를 호출하면 이 부분이 처리됩니다. server(stub) API의 경우 cancel()이 추가된다면 내부구현에서 native handle을 제거하고 다시 만드는 형태가 되어야 될 것 같습니다. 일관성을 위해 client(proxy) API도 이런 형태로 구현되어도 될 것 같습니다. 이 방법이 아니라면 native API를 추가하고 사용하는 구조가 되어야 될 것 같습니다.

의견 부탁드리겠습니다.

swift-kim commented 1 year ago

hot restart 설명을 읽어보면 앱 상태를 잃어버린다고 되어있는데 Dart VM이 앱을 재시작 시켜도 기존 앱 리소스가 끝까지 정리 안되는 것 같습니다. 이 부분은 Dart VM의 버그일까요? 동작이 기대한 것(이후에 리소스 정리)과 달라서 한번 언급드려 봅니다.

음..정확히 어떤 말씀이신지 잘 모르겠습니다. 앱의 state를 잃는 것과 앱의 native 리소스의 해제는 연관 관계가 없어 보입니다. widget 안에서 사용된 리소스가 있다면 dispose에서 해제를 해주어야 합니다.

client(proxy) API의 경우 cancel()의 추가는 어려운 상황은 아닌 것 같습니다. 내부적으로 disconnect() API를 호출하면 이 부분이 처리됩니다.

client 측 API에는 이미 disconnect가 있으므로 API 추가는 불필요해 보입니다. 앱의 dispose에서 disconnect를 불러준다면 올바른 형태가 되겠습니다.

그와 별개로 궁금한 것은 현재 TIDL 인터페이스에 정의된 인터페이스의 클래스 구현체에 connectdisconnect가 만들어지는데 만약 여러 개의 인터페이스가 정의되어 있다면 각각의 구현체에 connect가 생기게 되나요? (잠깐 gRPC의 C#와 Dart API를 봤는데 client와 channel 개념을 분리해서 사용하고 channel에 대해서만 initialize와 shutdown을 하는 것 같군요.)

server(stub) API의 경우 cancel()이 추가된다면 내부구현에서 native handle을 제거하고 다시 만드는 형태가 되어야 될 것 같습니다. 일관성을 위해 client(proxy) API도 이런 형태로 구현되어도 될 것 같습니다. 이 방법이 아니라면 native API를 추가하고 사용하는 구조가 되어야 될 것 같습니다.

rpc_port_stub_listen의 반대에 해당하는 API가 없기 때문에 rpc_port_stub_destroy를 하고 다시 rpc_port_stub_create를 해야한다는 의미일까요? Native API를 추가하는 게 맞는 것 같지만 역시 Tizen 7.0에나 들어올 수 있기 때문에 그런 방식으로도 구현이 가능하다면 해 볼만 할 것 같습니다. Dart API가 변경된다면 어떤 식으로 변경하는 게 좋을지 고민이 필요하실 것 같습니다. gRPC나 HttpServer 등의 API도 참고해보시기 바랍니다.

hjhun commented 1 year ago

hot restart 설명을 읽어보면 앱 상태를 잃어버린다고 되어있는데 Dart VM이 앱을 재시작 시켜도 기존 앱 리소스가 끝까지 정리 안되는 것 같습니다. 이 부분은 Dart VM의 버그일까요? 동작이 기대한 것(이후에 리소스 정리)과 달라서 한번 언급드려 봅니다.

음..정확히 어떤 말씀이신지 잘 모르겠습니다. 앱의 state를 잃는 것과 앱의 native 리소스의 해제는 연관 관계가 없어 보입니다. widget 안에서 사용된 리소스가 있다면 dispose에서 해제를 해주어야 합니다.

Hot restart를 반복해보면 리소스가 새로 할당되서 동작하게 됩니다. 이 동작을 반복하면 app process는 자신이 할당 가능한 메모리 영역까지 리소스를 반복적으로 할당하게 되어서 혹시나해서 말씀드려봤습니다. 최초 실행으로 할당된 server instance가 새로 시작된 앱으로 인해 더 이상 사용되지 않는걸로 인지되어 Dart VM에서 해제해야되는게 아닐까하는 접근으로 시작된 궁금증이였습니다.

client(proxy) API의 경우 cancel()의 추가는 어려운 상황은 아닌 것 같습니다. 내부적으로 disconnect() API를 호출하면 이 부분이 처리됩니다.

client 측 API에는 이미 disconnect가 있으므로 API 추가는 불필요해 보입니다. 앱의 dispose에서 disconnect를 불러준다면 올바른 형태가 되겠습니다.

그와 별개로 궁금한 것은 현재 TIDL 인터페이스에 정의된 인터페이스의 클래스 구현체에 connectdisconnect가 만들어지는데 만약 여러 개의 인터페이스가 정의되어 있다면 각각의 구현체에 connect가 생기게 되나요? (잠깐 gRPC의 C#와 Dart API를 봤는데 client와 channel 개념을 분리해서 사용하고 channel에 대해서만 initialize와 shutdown을 하는 것 같군요.)

각 인터페이스에 대해 connect()가 생성됩니다. proxy의 connect가 호출되어 server(stub)와 연결이 되면 이때 port가 생성됩니다. port의 disconnect를 호출하게 되면 server와의 연결이 해제 됩니다.

server(stub) API의 경우 cancel()이 추가된다면 내부구현에서 native handle을 제거하고 다시 만드는 형태가 되어야 될 것 같습니다. 일관성을 위해 client(proxy) API도 이런 형태로 구현되어도 될 것 같습니다. 이 방법이 아니라면 native API를 추가하고 사용하는 구조가 되어야 될 것 같습니다.

rpc_port_stub_listen의 반대에 해당하는 API가 없기 때문에 rpc_port_stub_destroy를 하고 다시 rpc_port_stub_create를 해야한다는 의미일까요? Native API를 추가하는 게 맞는 것 같지만 역시 Tizen 7.0에나 들어올 수 있기 때문에 그런 방식으로도 구현이 가능하다면 해 볼만 할 것 같습니다. Dart API가 변경된다면 어떤 식으로 변경하는 게 좋을지 고민이 필요하실 것 같습니다. gRPC나 HttpServer 등의 API도 참고해보시기 바랍니다.

네. rpc_port_stub_listen()의 반대에 해당하는 API가 없기 때문에 rpc_port_stub_destroy()를 하고 다시 rpc_port_stub_create()를 해야된다는 의미였습니다. rpc-port의 디자인에서 listen에 반대되는 구현부를 두지 않은 이유는 더 이상 사용하지 않을 때 instance를 제거해서 사용하게 하려는 의도였습니다. @upple 님이 최초에 패치를 올렸을 때에는 dispose()가 있었지만 디자인상의 문제로 이 부분이 제거가 되었던걸로 기억합니다. gRPC를 보면 Server쪽에도 shutdown()이 존재합니다. 제가 생각하는 수정안은 rpc-port에서는 listen()에서 native handle을 생성해서 사용하게하고 close()에서 native handle을 제거하는 구조가 되면 될 것 같습니다.

hjhun commented 1 year ago

사용성 측면을 생각해보면 StubBase를 생성할 때 listen까지 해출해주는 구조가 좋을것 같습니다. 다만 이렇게 되면 지금의 구현을 전부 수정해야되는 상황이 됩니다. trusted, privileges arg를 전달받는 constructor가 필요하게 됩니다.

close 또는 shutdown이 호출되게 되면 native handle을 제거해서 처리할 수 있을 것 같습니다.

hjhun commented 1 year ago

API를 추가했을 때 Hot restart때 어떤 시점에 호출해주게 하면 될까요? main()이 종료되는 시점이면 되는걸까요? API를 추가했음에도 Hot restart때 처리를 못하는게 아닌지 약간 우려스러워서 질문드려봅니다. 우리가 Finalizer를 사용하는게 지금 상황에서 의미가 없다면 추가하는 API를 넣어도 동일한게 아닐까하는 우려가 있습니다. 지금과 같이 Dart VM이 동작한다면 기존 connection이 남아있어서 여전히 문제가 될 것 같습니다. 제가 잘못 생각하는 것일수도 있지만 우려가되어서 코멘트에 남겨봅니다.

항상 좋은 의견과 답변 감사드립니다.

swift-kim commented 1 year ago

이 동작을 반복하면 app process는 자신이 할당 가능한 메모리 영역까지 리소스를 반복적으로 할당하게 되어서 혹시나해서 말씀드려봤습니다.

재시작을 반복하다보면 자동적으로 GC가 수행될 것 같습니다.

API를 추가했을 때 Hot restart때 어떤 시점에 호출해주게 하면 될까요? main()이 종료되는 시점이면 되는걸까요?

Flutter 쪽 이슈들을 (https://github.com/flutter/flutter/issues/10437) 살펴봤는데 hot restart 시점을 Dart 코드 상에서 알 수 있는 방법이 없는 것 같습니다. 저는 dispose가 불릴 것으로 예상했지만 실제로 불리지 않고 유일하게 리소스를 정리할 수 있는 시점은 플러그인 native 구현에서 StreamHandlerOnCancel 내부인 것 같습니다. 그래서 일단 dispose에서 명시적으로 cancel하는 경우와 hot restart의 경우를 분리해서 생각해야 하겠고, hot restart 상황에서 적절해 보이는 해결책은

  1. OnListen 시에 handle 값을 멤버 변수로 저장해두고
  2. OnCancel 시에 해당 handle을 사용하여 rpc_port_stub_destroy를 실행

인 것 같습니다. 다른 방법으로는 (1) 아예 hot restart 상황을 지원하지 않는 것과 (2) handle 생성 자체를 Dart가 아닌 native 코드 안에서 하는 것이 있습니다 (https://github.com/flutter-tizen/plugins/issues/504#issuecomment-1365639841 와 같은 맥락).

위와 같이 구현하면 dispose에서 명시적으로 cancel하는 경우도 쉽게 구현이 될 것입니다. StubBase 클래스에 cancel 혹은 close 메서드를 만들고

_streamSubscription?.cancel();

만 해주면 native 구현의 OnCancel이 실행되겠지요. 기존 API에 변경은 없고 cancel 혹은 close만 추가되므로 큰 변경사항은 아닙니다.

제가 휴가라서 괜찮으시다면 다음주에 구현을 해 보고 동작에 문제가 없는지 말씀드리겠습니다. (StubBase가 두 개 이상인 상황을 고려해야 하겠네요.) 혼동을 피하기 위해 https://github.com/flutter-tizen/plugins/pull/505 가 먼저 merge되면 좋을 것 같습니다.

hjhun commented 1 year ago

휴가중이신데 친절한 설명 감사드립니다.

swift-kim commented 1 year ago

몇 가지 실험을 해봤는데 위에서 얘기한 것처럼 OnCancel에서 rpc_port_stub_destroy를 실행하려고 하면 알 수 없는 crash가 발생하고

Thread 1 "Runner.dll" received signal SIGABRT, Aborted.
0xb771aa51 in __kernel_vsyscall ()
(gdb) bt
#0  0xb771aa51 in __kernel_vsyscall ()
#1  0xb727f246 in __libc_signal_restore_set (set=0xbfa278d0) at ../sysdeps/unix/sysv/linux/internal-signals.h:84
#2  __GI_raise (sig=6) at ../sysdeps/unix/sysv/linux/raise.c:48
#3  0xb72663ce in __GI_abort () at abort.c:79
#4  0xb72c5a67 in __libc_message (action=<optimized out>, fmt=<optimized out>, fmt@entry=0xb73e00cb "*** %s ***: %s terminated\n") at ../sysdeps/posix/libc_fatal.c:181
#5  0xb73680a5 in __GI___fortify_fail_abort (need_backtrace=need_backtrace@entry=false, msg=msg@entry=0xb73e00a9 "stack smashing detected") at fortify_fail.c:28
#6  0xb7368043 in __stack_chk_fail () at stack_chk_fail.c:29
#7  0x97d17082 in (anonymous namespace)::RpcStubStreamHandler::OnCancelInternal (this=0x8057e050, arguments=0x804d0430) at src/tizen_rpc_port_plugin.cc:143

코드도 난해해지는 문제가 있어 (handle의 생성은 Dart에서, destroy는 native에서) 당장은 hot restart 상황을 지원하지 않는 방향으로 결정했습니다. 대신 StubBase.close 메서드만 추가했습니다.

제가 생각하는 수정안은 rpc-port에서는 listen()에서 native handle을 생성해서 사용하게하고 close()에서 native handle을 제거하는 구조가 되면 될 것 같습니다.

Dart의 listen에서 handle을 생성하고 close에서 제거하는 방식을 적용해봤지만 한번 close를 하고 다시 listen을 하면 이전에 적용한 setTrustedaddPrivilege의 정보가 날아가는 문제가 있어 다시 revert 했습니다.

Native의 OnListen에서 handle을 생성하고 OnCancel에서 제거하는 것도 고려해봤지만 stub이 여러 개일 경우를 가정하면 각각을 위한 event channel을 생성해야하고 구현이 크게 변경되기 때문에 보류했습니다. (현재 구현이 stub이 여러 개일 경우에 대응 가능하다는 것은 아닙니다.) 그외에도 코드를 개선해보려고 했지만 어떻게 해도 코드가 크게 변경될 수밖에 없어서 적용되지 못했습니다.

일단 위에 언급된 문제들 중에 해결되지 않은 것들이 먼저 고쳐져야하지 않을까 싶습니다 (특히 2와 3-2).

upple commented 1 year ago

510

리뷰 받은 내용에 대해서 적용하였습니다. 추가로 sync method의 경우 async 키워드를 제거하였습니다.

swift-kim commented 1 year ago

가능하면 사용하신 TIDL interface도 source tree에 추가해주시구요 (example/message.tidl 혹은 example/interface/message.tidl 등)

추가로 네 가지 의문이 있습니다.

  1. TIDL interface에서 여전히 특정 method를 async로 지정할 수 있나요? 만약 그렇다면 어떤 Dart 코드가 생성되는지와 위의 문제가 해결되었는지 설명해주실 수 있을까요?
  2. register를 sync로 변경하셨는데 이 method는 callback을 인자로 받으니까 async가 맞지 않나요? sync라면 callback이 의미가 없을뿐 아니라 예제가 무엇을 의도한 것인지 알기 어려워질 것 같습니다.
  3. message_client.dart는 자동 생성된 코드가 맞나요? 왜 unregister의 경우에만 여전히 async가 붙어있나요?
  4. register, send, unregister에 뭔가 기능적인 의미가 있나요? 사용자가 예제를 처음 볼 경우 send 전후로 registerunregister가 있어야하는 것으로 오해하기 쉬운 것 같습니다. 예제의 의미를 쉽게 이해할 수 있도록 interface를 수정하는 건 어떤가요? (예: sync와 async 예시 하나씩)
upple commented 1 year ago

리뷰 감사합니다. 아래 답변 남겼습니다. 말씀주신 내용 참고해보겠습니다. 감사합니다.

가능하면 사용하신 TIDL interface도 source tree에 추가해주시구요 (example/message.tidl 혹은 example/interface/message.tidl 등)

추가로 네 가지 의문이 있습니다.

  1. TIDL interface에서 여전히 특정 method를 async로 지정할 수 있나요? 만약 그렇다면 어떤 Dart 코드가 생성되는지와 위의 문제가 해결되었는지 설명해주실 수 있을까요?

-> TIDL 기존 구현에서 async 키워드는 단방향 호출 여부에 따라 표시하는 구조였습니다. 따라서 현재 발생한 block이슈는 해당 수정과는 관련이 적으며, 불필요한 async 키워드를 제거했다고 이해하시면 좋을 것 같습니다. 위에서 논의한대로 해당 이슈를 해결하려면 client핸들을 제대로 dispose하거나 별도의 스레드를 통해서 해당 작업을 수행해야 해결 될 것으로 보입니다.

  1. register를 sync로 변경하셨는데 이 method는 callback을 인자로 받으니까 async가 맞지 않나요? sync라면 callback이 의미가 없을뿐 아니라 예제가 무엇을 의도한 것인지 알기 어려워질 것 같습니다.

-> 위 답변에서 말씀 드렸다시피 TIDL에서의 async 키워드는 단방향 호출에 의미로 사용된 것으로 callback 유무와는 관련이 없습니다. 말씀주신 것처럼 의미적으로 혼동이 올 수 있다는 내용은 동감합니다.

  1. message_client.dart는 자동 생성된 코드가 맞나요? 왜 unregister의 경우에만 여전히 async가 붙어있나요?

-> 넵 현재 예제로 사용한 unregister에는 async 키워드가 붙어있습니다. 명일에 사용한 tidl 인터페이스 추가하겠습니다.

  1. register, send, unregister에 뭔가 기능적인 의미가 있나요? 사용자가 예제를 처음 볼 경우 send 전후로 registerunregister가 있어야하는 것으로 오해하기 쉬운 것 같습니다. 예제의 의미를 쉽게 이해할 수 있도록 interface를 수정하는 건 어떤가요? (예: sync와 async 예시 하나씩)

-> 기능적인 의미는 없습니다. 단지 인터페이스에서 정의된 형태입니다. 말씀주신대로 예제의 의미를 전달하기 쉽게 수정하는 방향을 내부적으로 논의해보겠습니다.

upple commented 1 year ago

@swift-kim 말씀주신대로 사용한 interface 추가하였습니다. 참고로 아래 tidl 패치를 통해 생성한 파일에서 주석만 제거하였습니다. https://review.tizen.org/gerrit/c/platform/core/appfw/tidl/+/286511