| d982f3183750046eef2d6be6548d507018944221 |
|
14-Mar-2017 |
Kazuhiro Inaba <kinaba@google.com> |
DO NOT MERGE: Revert "Remove StuckServer using backlog and tests relying on it"
This reverts commit 8ffb0f557712e57c7dbf795b2cff79ee61c51c59.
(DO NOT MERGE because the original CL was a cherry-pick form N,
where this wasn't a problem. It breaks trees only on M.)
Bug: 35934294
(cherry picked from commit d7f172d08f54daed09c1906f24eff3e68cd50c05)
Change-Id: Id0b18389de73110f3f07f591f4272dbb50b4417b
|
| 5dcc3ce8cae450f7800570b2826f5232096159cd |
|
20-Jan-2017 |
Yi Kong <yikong@google.com> |
DO NOT MERGE: Remove StuckServer using backlog and tests relying on it
This cherry-pick contains additional changes to remove more references
to StuckServer. The URLConnectionTest#teestConnectTimeouts() test is
suppressed in M via libcore/expectations/brokentests.txt and is not
expected to work. This change should not be merged into N branches.
(Original commit description follows below.)
StuckServer uses kernel implementation details that have changed on
version > 4.4, and causing tests depending on it to fail. Since we can't
find an alternative method to simulate a stuck server without special
permission, and the test itself is not reliable in that they don't fail
on non-stuck servers, remove them from Libcore tests.
Test: CtsLibcoreTestCases
Test: rm -rf out; m -j30 cts
Bug: 32551747
Bug: 35934294
(cherry picked from commit b5c7cd37726154f8da123e1844b5d60a63a60a2c)
Change-Id: I4dd951e9527b5ee5701c0c3ad3eaa0c6f34e8ef7
|
| d7f172d08f54daed09c1906f24eff3e68cd50c05 |
|
14-Mar-2017 |
Kazuhiro Inaba <kinaba@google.com> |
DO NOT MERGE: Revert "Remove StuckServer using backlog and tests relying on it"
This reverts commit 8ffb0f557712e57c7dbf795b2cff79ee61c51c59.
(DO NOT MERGE because the original CL was a cherry-pick form N,
where this wasn't a problem. It breaks trees only on M.)
It didn't block 'm cts' but it broke other targets.
libcore/luni/src/test/java/libcore/java/net/URLConnectionTest.java
still had a reference to StuckServer.
Change-Id: If9486b5b552c4992536507da5460436fb8185df3
|
| 8ffb0f557712e57c7dbf795b2cff79ee61c51c59 |
|
20-Jan-2017 |
Yi Kong <yikong@google.com> |
Remove StuckServer using backlog and tests relying on it
StuckServer uses kernel implementation details that have changed on
version > 4.4, and causing tests depending on it to fail. Since we can't
find an alternative method to simulate a stuck server without special
permission, and the test itself is not reliable in that they don't fail
on non-stuck servers, remove them from Libcore tests.
(cherry picked from commit 6a5e142b16cf6b921420d371c35d950a13b22db1)
Test: CtsLibcoreTestCases
Bug: 32551747
Change-Id: I88d837329d65040a9f8550fff790f8703ea773f3
(cherry picked from commit b5c7cd37726154f8da123e1844b5d60a63a60a2c)
|
| b5c7cd37726154f8da123e1844b5d60a63a60a2c |
|
20-Jan-2017 |
Yi Kong <yikong@google.com> |
Remove StuckServer using backlog and tests relying on it
StuckServer uses kernel implementation details that have changed on
version > 4.4, and causing tests depending on it to fail. Since we can't
find an alternative method to simulate a stuck server without special
permission, and the test itself is not reliable in that they don't fail
on non-stuck servers, remove them from Libcore tests.
(cherry picked from commit 6a5e142b16cf6b921420d371c35d950a13b22db1)
Test: CtsLibcoreTestCases
Bug: 32551747
Change-Id: I88d837329d65040a9f8550fff790f8703ea773f3
|
| 6a5e142b16cf6b921420d371c35d950a13b22db1 |
|
20-Dec-2016 |
Yi Kong <yikong@google.com> |
Remove StuckServer using backlog and tests relying on it
StuckServer uses kernel implementation details that have changed on
version > 4.4, and causing tests depending on it to fail. Since we can't
find an alternative method to simulate a stuck server without special
permission, and the test itself is not reliable in that they don't fail
on non-stuck servers, remove them from Libcore tests.
Test: CtsLibcoreTestCases
Bug: 32551747
Change-Id: I88d837329d65040a9f8550fff790f8703ea773f3
|
| 173e4cf05284f4f96b0c21976186edfc949559cd |
|
25-May-2016 |
Narayan Kamath <narayan@google.com> |
Selector: Use poll based selector instead of an epoll based selector.
The OpenJDK epoll based selector suffers from a serious bug where it
can never successfully deregister keys from closed channels.
The root cause of this bug is the sequence of operations that occur when
a channel that's registered with a selector is closed :
(0) Application code calls Channel.close().
(1) The channel is "preClosed" - We dup2(2) /dev/null into the channel's
file descriptor and the channel is marked as closed at the Java level.
(2) All keys associated with the channel are cancelled. Cancels are
lazy, which means that the Selectors involved won't necessarily
deregister these keys until an ongoing call to select() (if any) returns
or until the next call to select() on that selector.
(3) Once all selectors associated with the channel deregister these
cancelled keys, the channel FD is properly closed (via close(2)). Note
that an arbitrary length of time might elapse between Step 0 and this step.
This isn't a resource leak because the channel's FD is now a reference
to "/dev/null".
THE PROBLEM :
-------------
The default Selector implementation on Linux 2.6 and higher uses epoll(7).
epoll can scale better than poll(2) because a lot of the state related
to the interest set (the set of descriptors we're polling on) is
maintained by the kernel. One of the side-effects of this design is that
callers must call into the kernel to make changes to the interest set
via epoll_ctl(7), for eg., by using EPOLL_CTL_ADD to add descriptors or
EPOLL_CTL_DEL to remove descriptors from the interest set. A call to
epoll_ctl with op = EPOLL_CTL_DEL is made when the selector attempts to
deregister an FD associated with a channel from the interest set (see
Step 2, above). These calls will *always fail* because the channel has
been preClosed (see Step 1). They fail because the kernel uses its own
internal file structure to maintain state, and rejects the command
because the descriptor we're passing in describes a different file
(/dev/null) that isn't selectable and isn't registered with the epoll
instance.
This is an issue in upstream OpenJDK as well and various select
implementations (such as netty) have hacks to work around it. Outside
of Android, things will work OK in most cases because the kernel has its
own internal cleanup logic to deregister files from epoll instances
whenever the last *non epoll* reference to the file has been closed -
and usually this happens at the point at which the dup2(2) from Step 1
is called. However, on Android, sockets tagged with the SocketTagger
will never hit this code path because the socket tagging implementation
(qtaguid) keeps a reference to the internal file until the socket
has been untagged. In cases where sockets are closed without being
untagged, the tagger keeps a reference to it until the process dies.
THE SOLUTION :
--------------
We switch over to using poll(2) instead of epoll(7). One of the
advantages of poll(2) is that there's less state maintained by the
kernel. We don't need to make a syscall (analogous to epoll_ctl)
whenever we want to remove an FD from the interest set; we merely
remove it from the list of FDs passed in the next time we call
through to poll. Poll is also slightly more efficient and less
overhead to set up when the number of FDs being polled is small
(which is the common case on Android).
We also need to make sure that all tagged sockets are untagged before
they're preclosed at the platform level. However, there's nothing we
can do about applications that abuse public api (android.net.TrafficStats).
ALTERNATE APPROACHES :
----------------------
For completeness, I'm listing a couple of other approaches that were
considered but discarded.
- Removing preClose: This has the disadvantage of increasing the amount
of time (Delta between Step 0 and Step 3) a channel's descriptor is
kept alive. This also opens up races in the rare case where a
closed FD number is reused on a different thread while we have reads
pending.
- A Synchronous call to EPOLL_CTL_DEL when a channel is removed: This is a
non-starter because of the specified order of events in
AbstractSelectableChannel; implCloseSelectableChannel must be called
before all associated keys are cancelled.
bug: 28318596
(partially cherry picked from commit 4585ee7a9ef27260cb2e2b54bb18bc68861d5584)
This version of the change preserves the original EPoll classes because
they are used for asynchronous channels.
Change-Id: I61baf5b2403c7e793c167d99d46b28bee93f9ffc
|
| 4585ee7a9ef27260cb2e2b54bb18bc68861d5584 |
|
25-May-2016 |
Narayan Kamath <narayan@google.com> |
Selector: Use poll based selector instead of an epoll based selector.
The OpenJDK epoll based selector suffers from a serious bug where it
can never successfully deregister keys from closed channels.
The root cause of this bug is the sequence of operations that occur when
a channel that's registered with a selector is closed :
(0) Application code calls Channel.close().
(1) The channel is "preClosed" - We dup2(2) /dev/null into the channel's
file descriptor and the channel is marked as closed at the Java level.
(2) All keys associated with the channel are cancelled. Cancels are
lazy, which means that the Selectors involved won't necessarily
deregister these keys until an ongoing call to select() (if any) returns
or until the next call to select() on that selector.
(3) Once all selectors associated with the channel deregister these
cancelled keys, the channel FD is properly closed (via close(2)). Note
that an arbitrary length of time might elapse between Step 0 and this step.
This isn't a resource leak because the channel's FD is now a reference
to "/dev/null".
THE PROBLEM :
-------------
The default Selector implementation on Linux 2.6 and higher uses epoll(7).
epoll can scale better than poll(2) because a lot of the state related
to the interest set (the set of descriptors we're polling on) is
maintained by the kernel. One of the side-effects of this design is that
callers must call into the kernel to make changes to the interest set
via epoll_ctl(7), for eg., by using EPOLL_CTL_ADD to add descriptors or
EPOLL_CTL_DEL to remove descriptors from the interest set. A call to
epoll_ctl with op = EPOLL_CTL_DEL is made when the selector attempts to
deregister an FD associated with a channel from the interest set (see
Step 2, above). These calls will *always fail* because the channel has
been preClosed (see Step 1). They fail because the kernel uses its own
internal file structure to maintain state, and rejects the command
because the descriptor we're passing in describes a different file
(/dev/null) that isn't selectable and isn't registered with the epoll
instance.
This is an issue in upstream OpenJDK as well and various select
implementations (such as netty) have hacks to work around it. Outside
of Android, things will work OK in most cases because the kernel has its
own internal cleanup logic to deregister files from epoll instances
whenever the last *non epoll* reference to the file has been closed -
and usually this happens at the point at which the dup2(2) from Step 1
is called. However, on Android, sockets tagged with the SocketTagger
will never hit this code path because the socket tagging implementation
(qtaguid) keeps a reference to the internal file until the socket
has been untagged. In cases where sockets are closed without being
untagged, the tagger keeps a reference to it until the process dies.
THE SOLUTION :
--------------
We switch over to using poll(2) instead of epoll(7). One of the
advantages of poll(2) is that there's less state maintained by the
kernel. We don't need to make a syscall (analogous to epoll_ctl)
whenever we want to remove an FD from the interest set; we merely
remove it from the list of FDs passed in the next time we call
through to poll. Poll is also slightly more efficient and less
overhead to set up when the number of FDs being polled is small
(which is the common case on Android).
We also need to make sure that all tagged sockets are untagged before
they're preclosed at the platform level. However, there's nothing we
can do about applications that abuse public api (android.net.TrafficStats).
ALTERNATE APPROACHES :
----------------------
For completeness, I'm listing a couple of other approaches that were
considered but discarded.
- Removing preClose: This has the disadvantage of increasing the amount
of time (Delta between Step 0 and Step 3) a channel's descriptor is
kept alive. This also opens up races in the rare case where a
closed FD number is reused on a different thread while we have reads
pending.
- A Synchronous call to EPOLL_CTL_DEL when a channel is removed: This is a
non-starter because of the specified order of events in
AbstractSelectableChannel; implCloseSelectableChannel must be called
before all associated keys are cancelled.
bug: 28318596
Change-Id: I407b06b4b538e19823ac27a4c9ae4c608a01a2ea
|
| 867e936310a119e2c36c0844ee363c8f69061c81 |
|
14-Aug-2015 |
Narayan Kamath <narayan@google.com> |
Tweak selector test.
We must not return 0 on EINTR. We must loop back and continue
back to epoll_wait.
Change-Id: I96cfee0ad313b6b05984a7fc073d30154ea02b5e
|
| fa542091e45db699a937c5ac0191194405107827 |
|
16-Dec-2014 |
Elliott Hughes <enh@google.com> |
Fix poll to never return EINTR.
Bug: 18759467
Change-Id: Ia5b97a55318b5990ad6b80d15641223aa4fb06f5
|
| 99a0c82619a88c6aea038757cf14090f5d33afeb |
|
27-Nov-2014 |
Neil Fuller <nfuller@google.com> |
Fix crash in selector.wakeup() with closed selector
Selector.wakeup() can throw an undeclared IOException
(from native code).
This is not compatible with the signature of wakeup(). In prior
Android releases no exception is thrown in this case.
This change reverts the behavior to the same as prior Android
releases.
Many thanks to diddysbestbud@ for the report.
Bug: https://code.google.com/p/android/issues/detail?id=80785
Bug: 18548071
(cherry picked from commit 1791f6be1bd2733babb0c862ad8509f4c847b48f)
Change-Id: I19ee879dcd783655d8a402e12855a5fa1f1eb90c
|
| 1791f6be1bd2733babb0c862ad8509f4c847b48f |
|
27-Nov-2014 |
Neil Fuller <nfuller@google.com> |
Fix crash in selector.wakeup() with closed selector
Selector.wakeup() can throw an undeclared IOException
(from native code).
This is not compatible with the signature of wakeup(). In prior
Android releases no exception is thrown in this case.
This change reverts the behavior to the same as prior Android
releases.
Many thanks to diddysbestbud@ for the report.
Bug: https://code.google.com/p/android/issues/detail?id=80785
Bug: 18548071
Change-Id: I5421e8a0ae1fdf2cde9cb635dae56b4fd02b6ac4
|
| 5d930cadc8f62aee5f18e7921296fe66a54f18ab |
|
24-Apr-2014 |
Elliott Hughes <enh@google.com> |
Groundwork towards making the Libcore.os functionality public.
Change-Id: Ie700aa16d91fba53fc5eb2555829cb74d84b12ad
|
| 814c09b0c5b6f3baef57e839a543cf0320707157 |
|
07-Feb-2014 |
Neil Fuller <nfuller@google.com> |
Merge "Fix CTS test failure under ART"
|
| 0bb999c4d83b41e9d65735c417cae212bfab7d61 |
|
07-Feb-2014 |
Neil Fuller <nfuller@google.com> |
Making SelectorTest.testInterrupted() clearer.
As per discussion on
https://android-review.googlesource.com/#/c/81450/
Change-Id: I98ddc5acc95258e4f3c2dbfd9dd2a9d5884ba16d
Bug: 12905142
|
| e37bfbcea35237c5c61f4d94788eafbc5f1f4ee8 |
|
07-Feb-2014 |
Neil Fuller <nfuller@google.com> |
Fix CTS test failure under ART
The Thread.interrupted() State from
SelectorTest.testInterrupted() was leaking into
SelectorTest.testManyWakeupCallsTriggerOnlyOneWakeup() causing
that test to fail when the CTS was run with the ART. Under
Dalvik the "interrupted" state was being reset, but it was
causing the test runner to run each subsequent test in its
own Dalvik instance (which probably slowed down the tests but
wasn't causing failures).
It is unclear why a Thread being in "interrupted" after a test
does not cause problems for ART in the same way it causes
problems for Dalvik.
Bug: 12905142
Change-Id: I677c1d46602d10e174ba3b6b8614529b5c902104
|
| e2351b133933cbf0356447b6623ca39fa6cb4963 |
|
24-Aug-2013 |
Elliott Hughes <enh@google.com> |
Fix three failing tests.
Increase an overly-tight timeout in SelectorTest.
Remove an incorrect assertion about a return type.
Remove an assertion testing fallback behavior of Currency.getDisplayName where
icu4c's behavior changes between releases and the RI doesn't match its own
documentation.
(cherry-pick of 8b3d7dfa1b50eb611b20824242f35eba2cfba46e.)
Bug: 10210895
Change-Id: Ia21b7c20056646cfc3e8cc784741ef2cbd10b7df
|
| 8b3d7dfa1b50eb611b20824242f35eba2cfba46e |
|
24-Aug-2013 |
Elliott Hughes <enh@google.com> |
Fix three failing tests.
Increase an overly-tight timeout in SelectorTest.
Remove an incorrect assertion about a return type.
Remove an assertion testing fallback behavior of Currency.getDisplayName where
icu4c's behavior changes between releases and the RI doesn't match its own
documentation.
Bug: 10210895
Change-Id: I6ec4fd571d810d4b08c703acaec2e7c9d6bf9d69
|
| 57656d21f772aacbe0d05e54b1274f4c58993a52 |
|
10-Jul-2013 |
Elliott Hughes <enh@google.com> |
Fix Selector to allow read and write at the same time.
Bug: https://code.google.com/p/android/issues/detail?id=57456
Change-Id: I29e6688aafce886803bbbd12793df3ab952459c8
|
| 0e1afa1091f74a6228c01d8c7a8eebf001efdc57 |
|
13-Oct-2012 |
Elliott Hughes <enh@google.com> |
Fix ConcurrentCloseTest flakiness.
We can't rely on consuming all the listen(2) backlog. For the tests we've
seen fail because they sometimes connect rather than time out, switch to
an unroutable address.
(cherry-pick of babccbf9e429c4c78aca24c205825ceaaf7d3f37.)
Bug: 6971145
Change-Id: Ibfa412ff1ad7da7e63842d0162cc67a706e2b27e
|
| babccbf9e429c4c78aca24c205825ceaaf7d3f37 |
|
13-Oct-2012 |
Elliott Hughes <enh@google.com> |
Fix ConcurrentCloseTest flakiness.
We can't rely on consuming all the listen(2) backlog. For the tests we've
seen fail because they sometimes connect rather than time out, switch to
an unroutable address.
Bug: 6971145
Change-Id: I259d31b1a15123bcd78c36849d5ed863d392ac20
|
| f7bd2a99f6f4024e9034300b30a13a2ea871aa97 |
|
12-May-2012 |
Elliott Hughes <enh@google.com> |
Work around poll(2) failing with EINTR.
This restores the gingerbread behavior, which is arguably pretty annoying too,
but it's what we've always done so less likely to be disruptive.
Bug: 6453247
Change-Id: I22635e36a37cb36cf2b22d1739ab6a28662c9188
|
| d3992b82602c2eef8e83ae7150f987cc9e0864fd |
|
11-May-2011 |
Elliott Hughes <enh@google.com> |
Move the selector tests into one file.
Change-Id: I95691beaf3cee3ef89f16001c1a0176da3817c54
|
| 4557728efb66c455a52b7669a8eefef7a9e54854 |
|
11-Aug-2010 |
Jesse Wilson <jessewilson@google.com> |
Moving tests to be under the libcore.* package.
This is indended to make it easier to run on VMs that restrict the packages
from which application classes can be loaded. For example, on the RI you need
to use the bootclasspath to load these tests.
Change-Id: I52193f35c5fcca18b5a3e1d280505b1e29b388af
|