Summary:
Versions of flipper from before 7th August 2018 don't return any response, so we don't store any connection_config.json file.
To keep the functionality the same as with up-to-date app versions, when this happens, store an empty config file ("{}").
Reviewed By: danielbuechele
Differential Revision: D9682885
fbshipit-source-id: fd580f6bba6b6b20135aa2c076be10e1eea0f8bc
Summary:
SonarWebSocketImpl has got pretty bloated. So I'm extracting all the file interaction out of it into
ConnectionContextStore. The purpose of this class is to provide all the context needed to establish a connection.
This makes SonarWebSocketImpl more functional and therefore testable.
Reviewed By: priteshrnandgaonkar
Differential Revision: D9540089
fbshipit-source-id: 0cd1d69f2b11eaf9f569245a2da14f85cc140427
Summary:
If you use a null eventbase, folly implicitly decides which one to run the futures in, for example
the timekeeper thread.
The only component that passes in event bases is sonar.cpp.
Reviewed By: priteshrnandgaonkar
Differential Revision: D9539443
fbshipit-source-id: 7fd7289257c84b039a7ac00b14f78bb271262480
Summary:
Android devices don't always know their own serial.
But we do a certificate exchange using adb from the desktop, so we can provide it in the response.
After this the client will provide it every time it connects, so we can do things like filter plugins by device id.
For apps that have already done cert exchange, they'll continue to use 'unknown' as their id until they do it again with an up to date version of sonar.
We can think about forcefully stopping that, but I haven't done it.
Reviewed By: danielbuechele
Differential Revision: D9481460
fbshipit-source-id: f8932699711ebbec4260fabe32f87e6cdff920f2
Summary: calling SonarClient::instance()->stop() without a connection will try to deref a null client_
Reviewed By: jknoxville
Differential Revision: D9325527
fbshipit-source-id: b0a81d6eb9d1375b74cbf10466f947e8a2de3b98
Summary:
Add a method to add error messages into failed steps. This will be surfaced in the diagnostic screen, to help the user
troubleshoot.
Reviewed By: passy
Differential Revision: D9333322
fbshipit-source-id: 5f1e55c3d71b19292dbc428aaecfbd41e7a75125
Summary:
It's expected that whenever not connected to a desktop, or when the sonar desktop app isn't running, we'll get
errors connecting. Isolate these so we can treat all other exceptions properly.
Reviewed By: passy
Differential Revision: D9333323
fbshipit-source-id: c9853ca84d1a04827ebf7bae0fe87859ca6110d1
Summary:
Adds a more structured representation of state updates than a multiline string.
Also changes the string form to omit [started] records.
Reviewed By: danielbuechele
Differential Revision: D9315503
fbshipit-source-id: 55b8f9572091fd42fe852c8d26a8f2a53f76c82a
Summary:
NOTE: Second push of the same commit. The original was reverted because of an incompatibility with
the rsocket version used in the OSS build, which is now fixed.
[Step 2 of a protocol change between desktop app and agent]
The flipper agent periodically tries to connect.
When it doesn't have the required certs, instead of trying to connect, it requests them from the desktop.
After requesting, it just continues the loop, trying to request.
The problem with that is
a) the desktop can take longer than one cycle to generate and provide the certs, meaning the agent will make overlapping requests, causing confusion and it to take longer than necessary.
b) the desktop can take less time than a retry cycle, but the agent will still wait before trying to connect.
Fixing a) by making the agent wait for a response from the desktop before continuing attempting to reconnect.
This means on the next connection attempt, it's guaranteed that the desktop is finished processing the CSR.
b) remains unfixed for now, but can be dealt with separately.
This changes the agent to use requestResponse, instead of fireAndForget and wait for a response from Flipper before continuing.
Also added a fallback to detect old versions of Flipper/Sonar and use the oldFireAndForget method in those cases.
Reviewed By: danielbuechele
Differential Revision: D9315946
fbshipit-source-id: 8058f7d43690ba609f16a61c0a9b40991e9e83cc
Summary:
This breaks in open source due to a missing rsocket symbol and
blocks our legocastle task.
Closes https://github.com/facebook/flipper/issues/224
Original commit changeset: e782b303b5e4
Reviewed By: jknoxville, danielbuechele
Differential Revision: D9289450
fbshipit-source-id: b780c300394f5793e95ef2fb6b0e6ba0150caf9a
Summary: I noticed from the diagnostics screen that onDisconnected was never being called when sonar disconnects.
Reviewed By: danielbuechele
Differential Revision: D9265562
fbshipit-source-id: afd070126c6ef02a98c8dbc6589b6f9b8b83a730
Summary:
[Step 2 of a protocol change between desktop app and agent]
The flipper agent periodically tries to connect.
When it doesn't have the required certs, instead of trying to connect, it requests them from the desktop.
After requesting, it just continues the loop, trying to request.
The problem with that is
a) the desktop can take longer than one cycle to generate and provide the certs, meaning the agent will make overlapping requests, causing confusion and it to take longer than necessary.
b) the desktop can take less time than a retry cycle, but the agent will still wait before trying to connect.
Fixing a) by making the agent wait for a response from the desktop before continuing attempting to reconnect.
This means on the next connection attempt, it's guaranteed that the desktop is finished processing the CSR.
b) remains unfixed for now, but can be dealt with separately.
This changes the agent to use requestResponse, instead of fireAndForget and wait for a response from Flipper before continuing.
Also added a fallback to detect old versions of Flipper/Sonar and use the oldFireAndForget method in those cases.
Reviewed By: passy
Differential Revision: D9179393
fbshipit-source-id: e782b303b5e441f7d6c7faa3e5acdcbfb51e5e9c
Summary: These will be displayed in the sonar diagnostics screen, for troubleshooting connection issues.
Reviewed By: danielbuechele
Differential Revision: D9150552
fbshipit-source-id: c6d65fba86e7564fbb004aaa7b0303a1d5952e5d
Summary: These will be displayed in the sonar diagnostics screen, for troubleshooting connection issues.
Reviewed By: passy
Differential Revision: D9117508
fbshipit-source-id: 6481f127b908fa539fe1fed1e268a28fa357d6f8
Summary:
Overall plan to modify Future<T>::then to be r-value qualified and use Future<T>::thenTry or Future<T>::thenValue.
The goal is to disambiguate folly::Future and to improve type and lifetime safety of Future and its methods.
4/n: Codemod:
* rvalue-future<T>.then(callable with operator()(not-a-try)) to rvalue-future<T>.thenValue(callable with operator()(not-a-try)).
* rvalue-future<T>.then(callable with operator()()) to rvalue-future<T>.thenValue(callable with operator()(auto&&)).
* rvalue-future<T>.then(callable with operator()(auto)) to rvalue-future<T>.thenValue(callable with operator()(auto)).
Applied to xplat.
Reviewed By: marshallcline
Differential Revision: D9133114
fbshipit-source-id: 30cc4f0480ca04b3abda54af3aafd9fc4dfdf0e0
Summary:
One design goal of sonar is to never cause the host app to crash or hang.
For this reason, we do all heavy work in a background thread.
If we detect that it's not running in it's own thread, just return so we don't hold up the caller.
Reviewed By: danielbuechele
Differential Revision: D8767288
fbshipit-source-id: e146cc2cfe5c3e62d12f527ff79f24c74873d4ff
Summary:
Having another attempt at removing this. It's unsafe because it in some cases executes the .then() task in the timekeeper thread, rather than the executor the original future is using.
When that default executor is an InlineExecutor, for example, you can get stack overflow.
I tried to remove this use before, but having the same thread used for both the sonar client itself and rsocket, meant that they entered a deadlock trying to connect.
Now that I've separated those jobs into separate threads, they can execute independently.
Reviewed By: danielbuechele
Differential Revision: D8748356
fbshipit-source-id: a1029ece2c7006ad7642cbf8aa59e692c76b19b2
Summary:
We currently give sonar one event base from java / obj-c code to use for scheduling tasks.
This causes a problem, because we schedule reconnect tasks on the event base, and then these tasks interact with rsocket which schedules it's own tasks.
The problem is that we're passing rsocket the same event base. So the reconnect code executes and blocks waiting for rsocket to connect.
But rsocket can never connect because it never executes because that thread is blocked, so we get deadlock.
This is the first step which just changes the interface to pass two event bases.
The consumers will be changed to pass in different threads next.
Reviewed By: danielbuechele
Differential Revision: D8748350
fbshipit-source-id: 481d6f23644f28fd0f1c786252605287019c999c
Summary:
delayedUnsafe() is unsafe because it disregards the executor you have specified, and uses the default one.
Depending on the context, this can sometimes be an InlineExecutor, and since some of the scheduled jobs, schedule instances of themselves, this can cause infinite recursion to occur.
Fixing this by using the safe variant of delayed, and also using a new instance of IOExecutor.
We need a new Executor for the Sonar loop, because if we use the same worker thread as is provided to RSocket, we get deadlock when we wait for rsocket to connect.
Reviewed By: danielbuechele
Differential Revision: D8617679
fbshipit-source-id: 51ab3224b93e774596a8799338e7391e2eb956cb
Summary:
There's an issue with folly's delayedUnsafe(), where it drops your executor and effectively
runs all futures inline, in this case instead of scheduling tasks for the future, it grows the stack indefinitely.
This fixes the issue by using the safe delayed(), which preserves the executor, so the jobs get shceduled on a different thread as intended.
Reviewed By: LeeHowes
Differential Revision: D8575956
fbshipit-source-id: c5b2ced43a70505c51883281f202ac947ae6723f
Summary:
When an app with sonar is run for the first time, the necessary files don't exist.
This is expected, so don't output an error, only error if they do exist but there's some other problem.
Reviewed By: emilsjolander
Differential Revision: D8350995
fbshipit-source-id: ff0a4f0e7a73848f6172c6108a0caee6efb43553
Summary:
If the app has an old CSR with data that is incompatible with sonar, we can't use it.
An example of this happening is when we moved the package name from organisation to common name in the certificate subject.
To get around this, always create a new one to guarantee it contains what we expect.
Reviewed By: emilsjolander
Differential Revision: D8350247
fbshipit-source-id: e53148fcddc47aa60e3daef5bbf36ce330a3b4e9
