Create and destroy JniClient for each connection attempt |
|||||
Issue descriptionVersion: M53, up to change 2bc751024e4ddf3f50594abdbf3c54b77df84562 OS: Android M (I believe it's an android version independent crash) What steps will reproduce the problem? -- Cannot guarantee to reproduce consistently. Though I reproduced twice. (1) Connect to host. (2) Kill host service by task manager or reinstall host binary. -- It's expected the client disconnected from the host at this point. (3) Reconnect to the host. What is the expected output? Connect successfully. What do you see instead? Crashed, with error message 'Chromoting has stopped working' Please use labels and text to provide additional information. With logcat, both crashes happened when jingle_session received service-unavailable errors. 06-05 14:01:23.698 29352 29372 E chromium: [0605/140123:ERROR:jingle_session.cc(335)] Received error in response to session-initiate message: "<cli:iq to="zijiehe@google.com/chromoting20DDE7FE" type="error" id="10499074388698573428" from="432bb39daef3fb133d5e5ba228ff5f32@chromoting.gserviceaccount.com/chromoting28958F95" xmlns:cli="jabber:client"><jingle sid="8726832406128401399" action="session-initiate" initiator="zijiehe@google.com/chromoting20DDE7FE" xmlns="urn:xmpp:jingle:1"><content name="chromoting" creator="initiator"><description xmlns="google:remoting"><standard-ice/><control transport="mux-stream" version="3"/><event transport="mux-stream" version="2"/><video transport="stream" version="2" codec="vp9"/><video transport="stream" version="2" codec="vp8"/><audio transport="mux-stream" version="2" codec="opus"/><audio transport="none"/><authentication method="spake2_pair" supported-methods="third_party_spake2_curve25519,third_party,pair_spake2_curve25519,spake2_pair,spake2_curve25519,spake2_hmac,spake2_plain"><eke-message>NGIzGRShSDxabfLiisnw3j+XrW7J2dXzQD5KdMUyc0fuM9fuBfxLZJtFCIztFhmoko3Zd7VoQD8=</eke-message><pairing-info client-id="ca45dd66-8dd5-4019-8ee9-18a5a925105d"/></authentication></description></content></jingle><cli:error code="503" type="cancel"><service-unavailable xmlns="urn:ietf:params:xml:ns:xmpp-stanzas"/></cli:error></cli:iq>". Terminating the session. 06-05 14:01:23.875 29352 29352 F chromium: [0605/140123:FATAL:jni_client.cc(48)] Check failed: !secret_fetcher_. It looks like a superficial issue is in JniClient::DisconnectFromHost, secret_fetcher_ is not reset. But after digging a little bit deeper, we always destroy the JniClient after disconnecting, i.e. we should not call JniClient::ConnectToHost of a single instance twice. But some code path may break this assumption.
,
Jun 5 2016
,
Jun 6 2016
When I did the refactorization, I assume the lifecycle of JniClient starts with ConnectToHost and ends with DisconnectFromHost. I'll double check this with Lambros tomorrow. If that's not the case then resetting secret_fetcher_ may fix this problem.
,
Jun 6 2016
As a fix for the problem I introduced with the refactorization, I submit a CL to reset the secret_fetcher_ in DisconnectToHost(). However, after chatting with Lambros, we agree that a JniClient should be created for each connection and ConnectToHost() should not be called twice for the same client. I will investigate on this problem.
,
Jun 6 2016
,
Jun 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/909aa6b2ac08abe5d75cdb5050c979c29ef451e3 commit 909aa6b2ac08abe5d75cdb5050c979c29ef451e3 Author: yuweih <yuweih@chromium.org> Date: Mon Jun 06 19:17:57 2016 [Remoting Android] Releases |secret_fetcher_| in DisconnectFromHost() In some situations the same JniClient will be used for multiple connections where ConnectToHost()->DisconnectFromHost()->ConnectToHost() will be called. In ConnectToHost() we DCHECK !secret_fetcher_ but don't release it in DisconnectFromHost(), which fails the DCHECK. This CL solves this problem by resetting |secret_fetcher_| in DisconnectFromHost(). However, we still need to investigate why JniClient is being used for multiple connections. BUG= 617471 Review-Url: https://codereview.chromium.org/2046663002 Cr-Commit-Position: refs/heads/master@{#398085} [modify] https://crrev.com/909aa6b2ac08abe5d75cdb5050c979c29ef451e3/remoting/client/jni/jni_client.cc
,
Jun 6 2016
What really happens in the repro is that when we restart the host, the host info on the client will become stale, and SessionConnector's (Java) first ConnectToHost call will fail, and it will refresh the host list and retry ConnectToHost. In this case we are not really reusing the same JniClient for multiple connection but rather like retrying when it fails. For robustness reason it may be a good idea to merge ConnectToHost and DisconnectFromHost into the Java client's ctor and destroy method and have JniClient created and destroyed for each connection attempt.
,
Jun 7 2016
,
Aug 20
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by zijiehe@chromium.org
, Jun 5 2016