Skip to content
This repository has been archived by the owner on Sep 2, 2020. It is now read-only.

Add a basic stack-tracking mechanism for easier debugging. #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bklimt
Copy link
Contributor

@bklimt bklimt commented May 4, 2014

No description provided.

@bnham
Copy link

bnham commented May 5, 2014

I think you could just use a regular "volatile boolean" here rather than an AtomicBoolean. The latter is useful if you need the CAS method, which this commit doesn't seem to use.

@depoll
Copy link

depoll commented May 6, 2014

I'd watch out for volatile boolean -- looks like volatile hasn't always been respected by the Dalvik VM (http://stackoverflow.com/questions/4588076/is-dalviks-memory-model-the-same-as-javas)

executor.execute(new Runnable() {
@Override
public void run() {
final ArrayList<StackTraceElement> previousStack = stackTrace.get();
stackTrace.set(currentStack);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment about the impact of setting the threadlocal here/what it's intended to do?

@depoll
Copy link

depoll commented May 6, 2014

Overall looking pretty good/useful -- just the comments listed above :)

@josephearl
Copy link

@bklimt are you intending to update this?

If not I will clone your branch, make the modifications and submit as a new PR.

@grantland
Copy link
Member

AFAIK he's not. I've also tried using this internally and I get OOM exceptions.

@josephearl
Copy link

Hmm OK, I'll give it a spin and some point see if it's worth continuing.

@grantland
Copy link
Member

*internally within our codebase that uses bolts, not just bolts on its own.

@grantland grantland mentioned this pull request Aug 20, 2015
@ghost
Copy link

ghost commented Jul 12, 2016

Thank you for your pull request. As you may know, we require contributors to sign our Contributor License Agreement, and we don't seem to have you on file and listed as active anymore. In order for us to review and merge your code, please email [email protected] with your details so we can update your status.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants