Skip to content

GitLab

Switch branch/tag
  1. 08 Jun, 2016 6 commits
  3. 07 Jun, 2016 4 commits
  5. 06 Jun, 2016 2 commits
  7. 01 Jun, 2016 1 commit
  9. 31 May, 2016 1 commit
  11. 27 May, 2016 5 commits
    • Narayan Kamath's avatar
      FileDispatcherImpl: Untag sockets before preClose. · 9666717e
      Narayan Kamath authored 
      We shouldn't close them after preClose because the descriptor
will then describe a different file (/dev/null).

bug: 28994821

(cherry picked from commit 48689e680a503e82cbe1d5cc122f4bb628aa2fa2)

Change-Id: I3c3519cb3a54fdebf6d51520747735af01664696
      9666717e
    • Joachim Sauer's avatar
      Merge "Sync default timezone to ICU." into nyc-dev · 75fc6a28
      Joachim Sauer authored
      75fc6a28
    • Przemyslaw Szczepaniak's avatar
      Merge "Fix java.util.zip.ZipFile with OPEN_DELETE." into nyc-dev · 09913b03
      Przemyslaw Szczepaniak authored
      09913b03
    • Narayan Kamath's avatar
      Selector: Use poll based selector instead of an epoll based selector. · 4585ee7a
      Narayan Kamath authored 
      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
      4585ee7a
    • Narayan Kamath's avatar
      OsTest: Explicitly delete temporary files. · 0c2a90d8
      Narayan Kamath authored 
      Don't rely on temporary directory cleanup.

bug: 28946760
Change-Id: I2cded2d0ffe6ea05008295460f8e16b09f8ad6f4
      0c2a90d8
  13. 26 May, 2016 3 commits
    • Przemyslaw Szczepaniak's avatar
      Fix java.util.zip.ZipFile with OPEN_DELETE. · a128ef7c
      Przemyslaw Szczepaniak authored 
      openJdk java.util.zip.ZipFile with OPEN_DELETE flag
uses unlink to remove the file just after it's opened.

This is causing trouble on /storage partition, we
can read/write unlinked file easily, but lseek seems
to fail with ENOENT.

This change introduces alternative implementation that
doesn't use unlink before closing the file.

Bug: 28901232
Bug: 28950284
Change-Id: I871a84e9a14bc1b4b9d5b0faa207579e27bcfc81
(cherry picked from commit ae6b1b85)
      a128ef7c
    • Joachim Sauer's avatar
      Sync default timezone to ICU. · f59264d3
      Joachim Sauer authored 
      When java.util.TimeZone.setDefault() is called (either by client code or
from ActivityThread.updateTimeZone due to ACTION_TIMEZONE_CHANGED) we
need to notify android.icu.util.TimeZone of this change, as it keeps a
cached android.icu.util.TimeZone object to represent that default value.

android.icu.util.TimeZone.setTimeZone would be the obvious candidate
here. Unfortunately that method was hidden to have a single consistent
way to set the timezone and tries to do some extra work that is
undesireable on Android.

To avoid potential loops of ICU setDefault calling JDK setDefault and
vice versa, we remove the ICU setDefault.

Bug: 28949992
Change-Id: I46a59551864cd2b780f72d28e01a0c39daf3aef5
      f59264d3
    • Neil Fuller's avatar
      Merge "Relax tests around HTTP header validation" into nyc-dev · 2276af34
      Neil Fuller authored
      2276af34
  15. 25 May, 2016 2 commits
    • Neil Fuller's avatar
      Relax tests around HTTP header validation · e62b4c58
      Neil Fuller authored 
      An associated change in OkHttp is relaxing the validation to
make it closer to the behavior in M. This may be temporary and
could be removed in a future release of Android.

Bug: 28867041
Bug: https://code.google.com/p/android/issues/detail?id=210205
Change-Id: If45f89578553e38ad455850f7f6bbecd4d51262e
      e62b4c58
    • Joachim Sauer's avatar
      Fix setCurrency resetting fraction digits. · 13659d2d
      Joachim Sauer authored 
      ICU DecimalFormat and java.text.DecimalFormat differ in their
definition of setCurrency in that the former is documented to leave the
minimum and maximum fraction digits untouched and the later updates it
to the default of the currency.

Since we implement java.text.DecimalFormat using the ICU DecimalFormat
we have to work around that difference.

Bug: 28893763
Change-Id: I0e7e19fe95f352e75cc9f53ad8a7739436de37d1
      13659d2d
  17. 24 May, 2016 16 commits