Commit graph

33 commits

Author SHA1 Message Date
Yabin Cui
7a3f8d6691 adb: clean up debug tracing a little.
Always use LOG() for debug tracing.
Remove useless D_lock. I believe it is useless to lock just before and after fprintf.

I verified the log output both on host and on device. The output looks fine to me.

Change-Id: I96ccfe408ff56864361551afe9ad464d197ae104
2015-09-02 20:21:00 -07:00
Yabin Cui
d6ab3c2245 adb: improve names for transport threads.
The old names seems confusing. output_thread was reading remote data and writing to
local sockets. input_thread was reading local sockets data and writing to remote.
This change tries to make it clear by renaming output_thread to read_transport thread,
and renaming input_thread to write_transport thread.

Change-Id: I2e7b4cde7a94d436f3745e9e3ab10780e7caa8ac
2015-08-31 13:34:27 -07:00
Elliott Hughes
0c3914a44d Distinguish transport threads.
Use "<-" and "->" to work around Linux's very short thread name limit.

Bug: http://b/23423333
Change-Id: I9f3f096415b47162dacb660cd0a97ac7ff6107e6
2015-08-31 10:46:05 -07:00
Siva Velusamy
49ee7cf9a1 adb: set thread names (linux & mac)
Bug: 23423333
Change-Id: I0069f32ddbae2a10fb130064f721facf45b2cc09
2015-08-31 07:52:52 -07:00
Yabin Cui
b329824e6c adb: clean up transport disconnect operations.
Move operations from global functions into member functions.
Add unit test.

Change-Id: Id4543d8e78541eb08c8e629f180c605c699737ec
2015-08-28 16:36:00 -07:00
Yabin Cui
12d7f0909a adb: call run_transport_disconnects() only once.
Currently run_transport_disconnects() are called twice. One is in
handle_offline(), another is before destroying transport.
The users of disconnect callback are listener, adb_auth_client, and
remote_sockets. All of them need only to be called once. And after
handle_offline, no new listeners, adb_auth_client, or remote_sockets
can be connected to the offlined transport. So I think we can remove
the second call to run_transport_disconnects().

Change-Id: I1ef8b6b7b5ab7ae1bad109be107c85973d65a2e3
2015-08-28 11:39:37 -07:00
Yabin Cui
f4b9928563 adb: disconnect: fix write-after-free memory corruption and crash.
Transport atransport objects are semi-reference counted: the input and
output threads each hold a reference. The adb disconnect command was
calling transport_unref to release a reference that it never had in the
first place. This meant that the refcount dropped to zero and the object
was deleted before either the input or output thread released its
reference. When that last thread released its reference, it wrote to
freed memory and also sometimes crashed.

This fix is to not release any unheld reference, instead it just kicks
the transport to break remote_read in output_thread. So all transport
close flow goes the following way:
output_thread (exit) -> main thread (offline the transport) ->
input thread (exit) -> main thread (destroy the transport)

Change-Id: Iad1fe718acc8716f3a79c8c22b426a1b2450452c
2015-08-27 14:41:07 -07:00
Yabin Cui
f306812303 Merge "adb: fix a fake data race on transport:kick reported by tsan." 2015-08-27 18:25:02 +00:00
Yabin Cui
d8459b3d3c adb: fix a fake data race on transport:kick reported by tsan.
It is reported by tsan as a double checked locking. But I think
it is not a real data race. Because I think the old code is able
to make sure t->kick() is only called once, and the caller of
kick_transport is not relying on the side-effect of calling
t->kick().
But as it is not perf critical, I don't mind breaking the double
checked locking pattern.

Bug: 23385662
Change-Id: Ie3597dd56bb514117c3865d2afcfd7c115731a78
2015-08-26 15:22:56 -07:00
Yabin Cui
34f45c5663 adb: keep all asocket operations in the main thread.
As far as I can see, all asockets operations happen in fdevent_loop()
in the main thread, excepting close_all_sockets(). Instead of adding
lock and ref_count for each asocket, a simpler way would be moving
close_all_sockets() from input_thread to the main thread.

In input_thread(), there are two path to break the loop and call
close_all_sockets(). One path is when receiving offline A_SYNC, which
is sent by the main thread. The other path is when read_packet
fails, which I believe is almost not possible and doesn't matter
(Because t->fd is closed just before t is freed.). So I move
close_all_sockets() to handle_offline() in the main thread.

the socket_list_lock in sockets.cpp could be removed. But I prefer
to leave it for the following changes.

Bug: 6558362
Change-Id: I5da23f60a67a331262c62693b9b127fe2689c799
2015-08-26 11:27:43 -07:00
Elliott Hughes
8fcd8bc046 EINTR is handled by adb_read/unix_read and friends.
No need to clutter the code with cases that won't happen.

Change-Id: I47c5a2e6170f902d2618719f44492234a4869567
2015-08-25 10:59:45 -07:00
Elliott Hughes
aa24549190 adb sync cleanup.
We can double the speed of "adb sync" (on N9) if we increase SYNC_DATA_MAX
from 64KiB to 256KiB. This change doesn't do that, because I still haven't
managed to plumb through the information about whether we're a new adb/adbd
to file_sync_client.cpp and file_sync_service.cpp. But this is already a big
change with a lot of cleanup, so let's do the cleanup and worry about the
intended change another day...

This change does improve performance somewhat by halving the number of
lstat(2) calls made on the client side, and ensuring that most packets are
sent with a single write. This has the pleasing result of making the null
sync on an AOSP N9 go from just over 300ms to around 100ms, which means it
now seems instantaneous (https://en.wikipedia.org/wiki/Mental_chronometry).

Change-Id: If9f6d4c1f93ec752b95f71211bbbb1c513045166
2015-08-24 11:15:01 -07:00
Dan Albert
1792c23cb8 Add feature list to connection banner.
This allows us to test for features explicitly rather than relying on
the protocol version number, allowing us to fall back gracefully if a
feature is not supported.

This will be needed for the upcoming shell upgrades for stdout/stderr
separation and exit code reporting.

Change-Id: Ibb1d8ad2611f7209901ee76d51346b453e9c5873
2015-08-14 08:24:37 -07:00
Yabin Cui
6264129434 Make clear of the data length sent by write_packet.
Change-Id: If51b4feaa19a710874999ad7e6131ff2e56a9f8a
2015-07-30 20:01:10 -07:00
Tamas Berghammer
3d2904cdf2 Increase size of the the adb packets sent over the wire
The reason behing this change is to increase the adb push/pull speed
with reduceing the number of packets sent between the host and the
device because the communication is heavily bound by packet latency.

The change maintains two way compatibility in the communication
protocol with negotiating a packet size between the target and the
host with the CONNECT packets.

After this change the push/pull speeds improved significantly
(measured from Linux-x86_64 with 100MB of data):

           | Old push | Old pull || New push  | New pull  |
-----------------------------------------------------------
Hammerhead | 4.6 MB/s | 3.9 MB/s || 13.1 MB/s | 16.5 MB/s |
-----------------------------------------------------------
Volantis   | 6.0 MB/s | 6.2 MS/s || 25.9 MB/s | 29.0 MB/s |
-----------------------------------------------------------
Fugu       | 6.0 MB/s | 5.1 MB/s || 27.9 MB/s | 33.2 MB/s |
-----------------------------------------------------------

Change-Id: Id9625de31266e43394289e325c7e7e473379c5d8
2015-07-22 13:06:06 -07:00
Spencer Low
8d8126a705 adb: logging: newlines, thread ids, error code overwriting
Add missing \n to uses of legacy D() macro. This should make the legacy
logging easier to read (and harder to miss important stuff).

On POSIX, use gettid() from libcutils instead of pthread_self() so that
the output shows a more reasonable number instead of a pointer value.
This should be ok since libbase's logging already uses gettid().

Win32:

Don't let the Win32 last error get overwritten by API calls after the
original error'ing API. When encountering an unknown error, log the
specific error code.

Change-Id: Ib8f72754efa7ba895d2f1cd914251fec2a1d894c
Signed-off-by: Spencer Low <CompareAndSwap@gmail.com>
2015-07-21 02:06:26 -07:00
Elliott Hughes
64d303a0e7 Fix adbd/adb server confusion in an error message.
Change-Id: Iacb8bcfb1e8e1d46198ee4ba8cf0ec0d1b98c75a
2015-07-18 13:57:46 -07:00
Elliott Hughes
e2d3677cc2 Improve the "device '(null)' not found" error.
Now we'll say "no devices found" if you haven't set ANDROID_SERIAL and
there's no device connected to default to.

Also clean up the relevant code a little.

Change-Id: Id254929629ce0888628d5ba8e67cd996ffbf9c8a
2015-06-24 10:36:41 -07:00
Dan Albert
c7915a3470 Make atransport be a real class.
Using non-POD types in atransport means we'll need to start treating
it as a real class (specifically with regards to new/delete rather
than malloc/free).

I've also cleaned up the home grown linked lists for transport_list
and pending_list to just be std::lists. We might want to refactor that
again to be an std::unordered_map keyed on serial, since that seems to
be a common way to search it.

Change-Id: I7f5e23cdc47944a9278099723ca029585fe52105
2015-05-21 15:49:05 -07:00
Dan Albert
dcd78a15d0 Make connection states a proper type.
Change-Id: I809f9b327c832b88dd63151bf7dcb012d88e81c4
2015-05-18 17:10:33 -07:00
Dan Albert
d99d902abd Be tolerant of devices that don't report serials.
The USB spec explicitly says this is optional, so we shouldn't be
relying on it.

Bug: http://b/20883914
Change-Id: Icf38405b00275199bcf51a70c47d428ae7264f2b
2015-05-06 16:48:52 -07:00
Elliott Hughes
9b0f354fa2 Simplify adb_thread_create.
Change-Id: I36d6021ef8fbc23e8bcd4ddbe1dac0eba467cc70
2015-05-05 13:41:21 -07:00
Elliott Hughes
3bd73c12c0 Give enum types CamelCase names for clarity.
Change-Id: I1c89f1cc155ee839f372fb14d972a288183b8bcd
2015-05-05 13:10:43 -07:00
Elliott Hughes
ab52c181fa Add WriteFdFmt and clean up more code.
Also say *which* device wasn't found.

Bug: http://b/20666660
Change-Id: I50e234ad89e39ae0a8995083c0b642c61275c5a3
2015-05-01 17:36:46 -07:00
Elliott Hughes
e67f1f87d9 More adb buffer fixes.
This patch factors out a lot of the basic protocol code: sending OKAY,
sending FAIL, and sending a length-prefixed string.

ADB_TRACE has been non-optional for a long time, so let's just remove
the #ifs.

Also actually build the device tracker test tool (and remove its duplicate).

Bug: http://b/20666660
Change-Id: I6c7d59f18707bdc62ca69dea45547617f9f31fc6
2015-05-01 15:55:37 -07:00
Elliott Hughes
6452a89aa8 More fixed-length buffer removal.
Bug: http://b/20666660
Change-Id: I0c738e9fed2defed48a9cf2d0a4f7b99c08dcf3d
2015-04-30 11:25:05 -07:00
Elliott Hughes
dc3b459ff9 Add missing null checks after allocations.
Bug: http://b/20317729
Change-Id: I62bb761d48ee59a1f4ddd0cdd0632432305ca2ca
2015-04-21 19:43:22 -07:00
Elliott Hughes
0b8ecb3290 Merge "Remove extern "C" barriers to using C++." 2015-04-17 17:31:51 +00:00
Elliott Hughes
2d4121c0dc Remove extern "C" barriers to using C++.
Change-Id: Ic046d6aa540738cb46b54531bc59ba3b47b0136d
2015-04-17 09:47:42 -07:00
Elliott Hughes
7be29c819b Show $ADB_VENDOR_KEYS if authentication fails.
Incorrectly set $ADB_VENDOR_KEYS is the most likely cause of failed
adb connections. Make it easier to debug such problems by including
the value in use in the error message.

Bug: 20165551
Change-Id: I64c1d98ae6d3fb40eea9e1f0ddcfcf4f2d9d7318
2015-04-16 22:54:44 -07:00
Elliott Hughes
09a45a1927 Fix "adb devices -l".
Change 055f1aa4ff switched to using isalnum(3)
but didn't take into account that isalnum has the opposite sense to the
function it replaced, so the tests should have been inverted.

Bug: http://b/20056546
Change-Id: I90630c0bea69ddbb4a95dc09f79f49d23fd497de
2015-04-03 16:12:15 -07:00
Dan Albert
3313426fad File header cleanup.
* sysdeps.h should always be included first.
 * TRACE_TAG needs to be defined before anything is included.
 * Some files were missing copyright headers.
 * Save precious bytes on my SSD by removing useless whitespace.

Change-Id: I88980e6e00b5be1093806cf286740d9e4a033b94
2015-03-19 15:32:33 -07:00
Dan Albert
bac3474a82 Move adb to C++.
I keep trying to clean things up and needing std::strings. Might as
well just do this now.

usb_linux_client.c is going to stay as C because GCC isn't smart
enough to deal with the designated initializers it uses (though for
some reason it is in C mode).

The Darwin files are staying as C because I don't have a way to test
that they build.

The Windows files are staying as C because while I can actually build
for them, it's slow and painful.

Change-Id: I75367d29205a9049d34460032b3bb36384f43941
2015-03-09 14:06:11 -07:00
Renamed from adb/transport.c (Browse further)