Obsolete
Status Update
No update yet.
Comments
su...@twofortyfouram.com
Err, I have no idea how gerrit changesets are reflected in the bug report, but there is a preliminary patch https://android-review.googlesource.com/#/c/159020/
en...@google.com
kr...@android.com
Scope of changes to bionic are somewhat more complex than initially thought.
The troublesome spots are mmap() and friends, malloc() and friends (due to symbol clash between struct, fieldname, and function names) as well as most of the routines in string.h
string.h exposes some issues, because the routines in it are both defined as inline functions that call out to assembly counterpart of the same name using an inconsistent __builtin_XXXX() convention. Sometimes the routines are available as _builtin___XXXX_chk and sometimes as __builtin_xxx.
I initially thought about limiting the macro magic purely for bionic/arm64, but there does not seem to be a nice way to do this.
Does this mean I need to touch all the .S files that are associated with string.h?
Even for the ones that are NOT for arm64?
Please take a look at:
https://code.google.com/p/android/issues/detail?id=180578
The troublesome spots are mmap() and friends, malloc() and friends (due to symbol clash between struct, fieldname, and function names) as well as most of the routines in string.h
string.h exposes some issues, because the routines in it are both defined as inline functions that call out to assembly counterpart of the same name using an inconsistent __builtin_XXXX() convention. Sometimes the routines are available as _builtin___XXXX_chk and sometimes as __builtin_xxx.
I initially thought about limiting the macro magic purely for bionic/arm64, but there does not seem to be a nice way to do this.
Does this mean I need to touch all the .S files that are associated with string.h?
Even for the ones that are NOT for arm64?
Please take a look at:
su...@twofortyfouram.com
Could you please reiterate why is it not possible to fix this in TSan? If the problem is some call happening before __tsan_init, we just need to call __tsan_init right there. Most (if not all) interceptors have this logic.
Does it still happen if __tsan_init is called from .preinit_array?
Does it still happen if __tsan_init is called from .preinit_array?
kr...@android.com
Its troublesome before __tsan_init because we lack __thread keyword support in bionic. I had to hack in a workaround in TSAN that does not use intercepted calls. (more on this below)
The majority of the problem actually occurs after __tsan_init has been called.
For example, pthread_create() calls a bunch of routines for the *new thread*, before it has had a chance to initialize its ThreadState. All of these routines running on the new thread are intercepted by tsan, and crashes because ThreadState has not been initialized yet.
A purely tsan-only fix is to "globally ignore" interceptors during troublesome times. This works fine for two threads, but doesn't scale to multiple threads - now you can miss events because they are being ignored.
I also tried putting in "global partial ignores" (i.e. ignore interceptors only on the new thread), but that got really hairy as well.
The fundamental problem is that TSAN is intercepting calls that it has no business intercepting.
For example, the same thing happens during thread join time, when one thread is being deconstructed.
Similar things happen during process exit time where ALL threads are being deconstructed.
Playing around with purely TSAN solution, all I managed to do was move around the actual place of the crash/hang/missed events.
The majority of the problem actually occurs after __tsan_init has been called.
For example, pthread_create() calls a bunch of routines for the *new thread*, before it has had a chance to initialize its ThreadState. All of these routines running on the new thread are intercepted by tsan, and crashes because ThreadState has not been initialized yet.
A purely tsan-only fix is to "globally ignore" interceptors during troublesome times. This works fine for two threads, but doesn't scale to multiple threads - now you can miss events because they are being ignored.
I also tried putting in "global partial ignores" (i.e. ignore interceptors only on the new thread), but that got really hairy as well.
The fundamental problem is that TSAN is intercepting calls that it has no business intercepting.
For example, the same thing happens during thread join time, when one thread is being deconstructed.
Similar things happen during process exit time where ALL threads are being deconstructed.
Playing around with purely TSAN solution, all I managed to do was move around the actual place of the crash/hang/missed events.
le...@googlemail.com
On x86_64/linux, precisely zero calls are intercepted during pthread_create() on the new thread until it has finished initializing its ThreadState.
That does not happen on aarch64/bionic
That does not happen on aarch64/bionic
nu...@gmail.com
Oh, the reason why I had to hack in a non-interceptible TLS workaround was that intercepted calls happen during pthread_key maintenance that occurs both during pthread_join time as well as at exit time.
What was happening when I was using pthread_get/setspecific() was that after a thread was destroyed, an intercepted call was GENERATING a NEW key for the dead thread. Hilarity ensued afterwards.
What was happening when I was using pthread_get/setspecific() was that after a thread was destroyed, an intercepted call was GENERATING a NEW key for the dead thread. Hilarity ensued afterwards.
ma...@gmail.com
Back to the topic at hand,
does anyone have any ideas on how to limit the bionic headerfile changes purely for aarch64?
https://code.google.com/p/android/issues/detail?id=180578
I *think* it might be better to limit the new internal symbols to 64bit arm parts, but there does not seem to be a nice way to do this (yet).
While my build currently "works", I don't think it will build for other platforms (i.e. 32bit arm, mips etc...)
Thanks
does anyone have any ideas on how to limit the bionic headerfile changes purely for aarch64?
I *think* it might be better to limit the new internal symbols to 64bit arm parts, but there does not seem to be a nice way to do this (yet).
While my build currently "works", I don't think it will build for other platforms (i.e. 32bit arm, mips etc...)
Thanks
lu...@gmail.com
It looks like __aarch64__ is defined for Clang builds. I am not sure about gcc, nor can I say that the bionic folks are going to be pleased with architecture-specific stuff polluting high-level headers.
ak...@gmail.com
@11: i think he's confused and expecting __aarch64__ to be defined in code that's built 32-bit.
st...@gmail.com
No, not confused (at least I think)
So the crux of the matter is this.
I need to touch some .S files, which are necessarily architecture specific, to add in the private symbols we need to support TSAN.
I can either touch ALL .S files (arm, arch64, x86 ...), or just the ones that gets used while building a 64 bit device (even if they are 32bit libraries)
Now, is it OKAY for me to touch all the .S files?
If that is the case, thenhttps://code.google.com/p/android/issues/detail?id=180578 is moot.
OTOH, If you guys want me to scope my changes purely for 64bit arm devices, then
https://code.google.com/p/android/issues/detail?id=180578 DOES matter IF there are ever any cases where 64bit library/executable calls out to 32bit library
I hope this is clear
So the crux of the matter is this.
I need to touch some .S files, which are necessarily architecture specific, to add in the private symbols we need to support TSAN.
I can either touch ALL .S files (arm, arch64, x86 ...), or just the ones that gets used while building a 64 bit device (even if they are 32bit libraries)
Now, is it OKAY for me to touch all the .S files?
If that is the case, then
OTOH, If you guys want me to scope my changes purely for 64bit arm devices, then
I hope this is clear
ha...@gmail.com
In other words,
if it is okay for me to touch all the variant memset.S and strlen.S files, then I do NOT need to have arch specific exceptions in the new .h files.
The downside is that its gonna be a bigger patch :-/
What do you guys think?
if it is okay for me to touch all the variant memset.S and strlen.S files, then I do NOT need to have arch specific exceptions in the new .h files.
The downside is that its gonna be a bigger patch :-/
What do you guys think?
zh...@gmail.com
Hi everyone.
I upload a set of patches, all tagged with this bug.
I have no idea if your gerrit handles stacked patches well. (It didn't before)
If it does not, I can always upload a merged patch.
The rough order of patches are
1. new files + pthreads + mman.h
2. system calls (all the .S files and the gensyscalls.py)
3. string.h and friends
4. malloc.h and friends
5. ioctl/tcgetattr
Its not readily apparent what the order of the patches should be.
Here are the change-ids:
Support for Thread Sanitizer for Bionic
Change-Id: I724fb8d77059d3e01e1c4336d1aa2c0c550bbb80
Add new system call weak aliases, modified so that it is not arm64 specific
Change-Id: I0011bd344ba9ce977e429081a0cb84d8e59463fc
Touching all of string.h
Change-Id: I826f328a1849331dc39d5ff6847be02aff2ff1ba
Now touching malloc/free and friends
Change-Id: I5a2d07bfcf332556aeb0c3734b5ba122c80ee567
Now touching ioctl/tcgetattr
Change-Id: Ia709a5e7acf22239ad57aa22c06a867da6f6ef14
I upload a set of patches, all tagged with this bug.
I have no idea if your gerrit handles stacked patches well. (It didn't before)
If it does not, I can always upload a merged patch.
The rough order of patches are
1. new files + pthreads + mman.h
2. system calls (all the .S files and the gensyscalls.py)
3. string.h and friends
4. malloc.h and friends
5. ioctl/tcgetattr
Its not readily apparent what the order of the patches should be.
Here are the change-ids:
Support for Thread Sanitizer for Bionic
Change-Id: I724fb8d77059d3e01e1c4336d1aa2c0c550bbb80
Add new system call weak aliases, modified so that it is not arm64 specific
Change-Id: I0011bd344ba9ce977e429081a0cb84d8e59463fc
Touching all of string.h
Change-Id: I826f328a1849331dc39d5ff6847be02aff2ff1ba
Now touching malloc/free and friends
Change-Id: I5a2d07bfcf332556aeb0c3734b5ba122c80ee567
Now touching ioctl/tcgetattr
Change-Id: Ia709a5e7acf22239ad57aa22c06a867da6f6ef14
ol...@gmail.com
Hi.
I just pushed up a new changeset that combines the 5 mentioned above.
https://android-review.googlesource.com/#/c/162104/
(Apparently gerrit still can't handle stacked changesets without manual intervention.
It marked the 5 prior patches as "unmergeable")
I just pushed up a new changeset that combines the 5 mentioned above.
(Apparently gerrit still can't handle stacked changesets without manual intervention.
It marked the 5 prior patches as "unmergeable")
[Deleted User] <[Deleted User]>
sk...@gmail.com
Thank you for your feedback. We assure you that we are doing our best to address the issue reported, however our product team has shifted work priority that doesn't include this issue. For now, we will be closing the issue as won't fix obsolete. If this issue currently still exists, we request that you log a new issue along with latest bug report here https://goo.gl/TbMiIO .
he...@gmail.com
If you handle all your notifications in a Service. In that Service, register a BroadcastReceiver with a IntentFilter for the Intent.ACTION_PACKAGE_CHANGED action and data-scheme "package". Then in onReciever, just re-display the Notifications that the buggy NotificationManagerService just canceled.
[Deleted User] <[Deleted User]>
Instead of activating/deactivating the component (the BroadcastReceiver class) at runtime, you might just get away with enabling a BroadcastReceiver at runtime with registerReceiver(BroadcastReceiver, IntentFilter). Without an entry in the manifest.
This saves you from the madness of creating a notification service that listens for Intent.ACTION_PACKAGE_CHANGED, has to be autorun and/or stopped on the fly to save ressources, and much more trouble.
PS: removing all services' notifications because only one (1!) service's component in my process was disabled, is a bug in my book, too.
This saves you from the madness of creating a notification service that listens for Intent.ACTION_PACKAGE_CHANGED, has to be autorun and/or stopped on the fly to save ressources, and much more trouble.
PS: removing all services' notifications because only one (1!) service's component in my process was disabled, is a bug in my book, too.
ax...@gmail.com
prince of tennis courts and the other "+1
[Deleted User] <[Deleted User]>
[Comment deleted]
dd...@gmail.com
The issue is of course broader than service notifications. I can reproduce this on CM 12.1 (based on Android 5.1.1). When I disable (or set back to default) one of several activities, my entire app dies when Intent.ACTION_PACKAGE_CHANGED is broadcast. The user is on a separate settings activity where they have just toggled the option to disable a different component. Their UI dissappears a few seconds later.
sa...@google.com
Thank you for your feedback. We have tried our best to address the issue reported, however our product team has shifted work priority which doesn't include this issue. For now, we will be closing the issue as "Won't Fix (Obsolete)". If this issue still currently exists, we request that you log a new issue along with the latest bug report here: https://goo.gl/TbMiIO and reference this bug for context.
Description
Changing the component enabled state of any component will break running background services.
STEPS TO REPRODUCE
1. Create an app with an Android Manifest registered BroadcastReceiver and a foreground service (e.g. it calls startForeground() to display an ongoing notification)
2. Start the service and verify the notification icon appears
3. Programmatically set the BroadcastReceiver's state to be enabled or disabled (this will move the receiver's component state from DEFAULT to either ENABLED or DISABLED), like the following getPackageManager().setComponentEnabledSetting(new ComponentName(getApplicationContext(), TestReceiver.class), PackageManager.COMPONENT_ENABLED_STATE_ENABLED, PackageManager.DONT_KILL_APP);
4. Watch the service's notification icon
RESULTS
Actual: The service's ongoing notification icon disappears shortly after the BroadcastReceiver's component state is changed
Expected: The notification icon should not disappear
REGRESSION
This bug reproduces in the SDK 14 emulator. This bug does not reproduce on the SDK 10 emulator or on an SDK 12 device.
NOTES
This bug reproduces if any component--BroadcastReceiver, Service, etc.--enabled state is changed. This bug does not occur if no state change occurs (e.g. if a component is already ENABLED, then setting ENABLED again will not cause the bug to reproduce).