Track pending fdevents, and abort if we see an fdevent ready for
read/write for 5 minutes continuously.
Also, add a service that intentionally starts spinning, to test this.
Test: manually changed timeout to a minute, `adb raw spin`
Change-Id: Ibfa5e7e654996587f745887cb04987b982d79bed
Tracking fdevents by pointer is dangerous because an fdevent can be
destroyed and recreated at the same address. Add a monotonically
increasing id field to fdevent for this purpose.
Test: treehugger
Change-Id: I706b57a9e669290ef2db258f85489a5155fc1152
This adds fdsan deallocation sanitization to all fds monitored by
fdevent, which is most of the ones in adb.
Bug: http://b/79786774
Test: python test_device.py
Change-Id: I465804fdb0fd0ac019445900a30ba3403f5bf711
Remove fdevent_install and fdevent_remove in favor of using
fdevent_create and fdevent_destroy, so that we can put RAII types (i.e.
unique_fd) into fdevent without worrying about -Wexit-time-destructors
or structs that are freed instead of deleted.
Bug: http://b/79786774
Test: python test_device.py
Change-Id: I8471cc00574ed492fe1b196944976cdaae8b7cff
Make it so that we handle run_on_main_thread calls after regular socket
events, so that we can use it as a way to ensure we've processed all
pending socket events.
Test: adb_test
Test: wine adb_test.exe
Change-Id: Ic215c7fed19a8e1699e759970658b3775aa08c45
There exists no path through which a value other than -1 can be written
to the SHELL_EXIT_NOTIFY_FD.
Test: adb_test
Test: adbd_test
Test: python test_device.py
Change-Id: I0630c302ba06bc76917f0445aea75d2dbe1dc865
If we get a ton of fdevent_run_on_main_thread calls while running one
of the handlers, the socket might become full, which will result in a
deadlock in fdevent_run_on_main_thread when a write to the fd blocks
with the mutex taken. Resolve this by making the fd nonblocking, which
is safe because we always write after appending to the list, and read
before emptying the list, which guarantees that if the byte we write is
consumed, the std::function we appended will be run.
Bug: http://b/74616284
Test: adb_test
Test: python test_device.py
Change-Id: I29319bda2ad7b5a5cdcd91d1d0ddf39f7ab7d115
Add a function to run a function on the main thread, to allow fdevents
that depend on a blocking function to be registered.
Bug: http://b/37869663
Test: adb_test on linux
Change-Id: I84a0b372360420b7647057297b8f437e8afa874e
When device goes offline, user usually has to manually replug the
usb device. This patch tries to solve two offline situations, all
because when adb on host is killed, the adbd on device is not notified.
1. When adb server is killed while pushing a large file to device,
the device is still reading the unfinished large message. So the
device thinks of the CNXN message as part of the previous unfinished
message, so it doesn't reply and the device is in offline state.
The solution is to add a write_msg_lock in atransport struct. And it
kicks the transport only after sending a whole message. By kicking
all transports before exit, we ensure that we don't write part of
a message to any device. So next time we start adb server, the device
should be waiting for a new message.
2. When adb server is killed while pulling a large file from device,
the device is still trying to send the unfinished large message. So
adb on host usually reads data with EOVERFLOW error. This is because
adb on host is reading less than one packet sent from device.
The solution is to use buffered read on host. The max packet size
of bulk transactions in USB 3.0 is 1024 bytes. By preparing an at least
1024 bytes buffer when reading, EOVERFLOW no longer occurs. And teach
adb host to ignore wrong messages.
To be safe, this patch doesn't change any logic on device.
Bug: http://b/32952319
Test: run python -m unittest -q test_device.DeviceOfflineTest
Test: on linux/mac/windows with bullhead, ryu.
Change-Id: Ib149d30028a62a6f03857b8a95ab5a1d6e9b9c4e
Switch pthread_* to use the adb_thread_* abstractions to allow the fdevent
and socket tests to compile on Win32.
Bug: http://b/27105824
Change-Id: I6541bb1398780b999837e701837d7f86a5eee8ca
On exit, these destructors get invoked while other threads might
still be using them, potentially causing a crash, and definitely
causing tsan to report a race condition.
Bug: http://b/23384853
Change-Id: I94de55d22f97f4edd1d7cc1f34e8c1f8dfd56a5a
Allow adb to build using gcc by explicitly using global scope for the
type for pollfd. An alternative would be to rename the pollfd field to
different, but I did not have a better name in mind.
Change-Id: I7925df1dca7e1acc5a289256f228e5fc3755e86e
reverse_service() calls handle_forward_request(), which calls
functions in fdevent.cpp. fdevent functions is only supposed
to be called in the main thread.
Add check in fdevent.cpp to make sure all operations come from
main thread.
Bug: 25355808
Change-Id: Iceb9273f3056acc0713eaafe086ac950ca80ff4f
* fdevent_{set,add,del}()
* CHECK() that the fdevent is FDE_ACTIVE, otherwise the caller would
be waiting for events that will never arrive.
* Remove ~ from "fde->events &= ~events" to keep desired bits instead
of turning off desired bits.
* fdevent_call_fdfunc()
* CHECK(FDE_PENDING) since it should always be true if the fdevent was
on the pending list, or if fdevent_subproc_event_func() is faking
things. The goal is to try to avoid losing events.
Change-Id: I979c2fffa0b3d6b635488cde11dc544691193018
Signed-off-by: Spencer Low <CompareAndSwap@gmail.com>
It is possible that the adb server on host has many sockets in
CLOSE_WAIT state. To prevent socket leak, always enable POLLRDHUP
in fdevent.cpp to detect sockets in CLOSE_WAIT state.
Update LocalSocketTest unit tests:
Change half_close_with_packet to read_from_closing_socket, as reading
from a SHUT_WR socket is not needed in adb.
Change close_with_no_events_installed to close_socket_in_CLOSE_WAIT_state,
as the latter is more close to the real situation in use.
Bug: 23314034
Change-Id: Ice4f4036624e5584eab6ba5848e7f169c92f037f
Add has_write_error flag in asocket, so it will not wait on local_socket_closing_list
to write pending packets in local_socket_close(). Although it doesn't fix any problem,
it helps to make the code more stable.
Add a missing put_apacket() in error handling.
Add a check when adding local socket in local_socket_closing_list.
Bug: 23314034
Change-Id: I75b07ba8ee59b7f277fba2fb919db63065b291be
It is reported that the registered fd can be bigger than FD_SETSIZE, and can't be
handled by select(). By moving to poll(), we can remove the limitation.
Although we can't ignore the possibility that there is a fd leak, but we can
still make the potential bug more explicit by moving to poll().
We didn't move to epoll() because it is not supported on mac.
Bug: 23820751
Change-Id: Icb39329c4984f1fef749472c9e088682ee8c3444
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
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
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>
The code which triggers these events (via the SHELL_EXIT_NOTIFY_FD) are
only called from code which is already guarded by #if !ADB_HOST.
Change-Id: I184414f5e090c1f08ee117e4c8c434cd4a8b5221
One of my build aliases doesn't play nice with USE_MINGW=1, so my build lied to me. Will revert until I fix it up.
This reverts commit 459df8f3a1.
Change-Id: I7905c5ae5ee85fb2d228ce63d81c79f140998c18
Apparently there are two classes of this warning in clang.
-Wformat-security is only emitted for cases of
`func(nonliteral_fmt_string)` (no args), and -Wformat-nonliteral is
emitted for cases *with* arguments. For whatever reason, the latter
isn't included in -Wextra and must be manually enabled.
To make this more easily portable to Windows, move the existing
gnu_printf/__printf__ decision into base/macros.h as ATTRIBUTE_FORMAT.
Change-Id: I3b0990e1d1f0a2e9c13b32f5cd60478946cb5fc6
* 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
if many jdwp connection are created(), the memory will be leaked.
When it deletes heap memory on jdwp_process_free(),
the proc->fde just set to null.
so it need to free() in fdevent_destory().
Renamed readx/writex to ReadFdExactly/WriteFdExactly respectively.
These read/write a full fixed-size buffer. If the whole buffer cannot
be read/written, these functions return an error.
Rename write_string to WriteStringFully.
Move the TEMP_FAILURE_RETRY definition in sysdeps.h out of the
!Windows section. It seems Windows won't actually interrupt a call,
but it's easier to just define it than to #ifdef each call.
Change-Id: Ia8ddffa2a52764a2f9a281c96c937660e002b9b9
Much of adb is duplicated in bootable/recovery/minadb and fastboot.
Changes made to adb rarely get ported to the other two, so the trees
have diverged a bit. We'd like to stop this because it is a
maintenance nightmare, but the divergence makes this difficult to do
all at once. For now, we will start small by moving common files into
a static library. Hopefully some day we can get enough of adb in here
that we no longer need minadb.
Bug: 17626262
Change-Id: Ic8d5653bfcc0fec4e1acbece124402355084b864