Office Hours — Today, October 5

← Yesterday, October 4

Oct 5
7:20 PM
Mark M.
has entered the room
Mark M.
turned on guest access
7:25 PM
Steve S.
has entered the room
Steve S.
Hi, Mark!
Mark M.
hi, Steve!
how can I help you today?
Steve S.
I'm trying to create a timer that displays text showing how long in the past something happened
i have code that compiles and runs, but i want to make sure there are no memory leaks and it can run on the main thread
Here's the code:
View paste (6 more lines) 
static private class TimeCompleteHandler extends Handler {
    static final private long PERIOD_MILLIS = 1000;
    final private WeakReference<TextView> mTextAgo;

    TimeCompleteHandler(TextView tv) {
        mTextAgo = new WeakReference<TextView>(tv);
    }

    void start() {
        postDelayed(new Runnable() {
            private int mCounter = 0;

            @Override
            public void run() {
                if (mTextAgo.get() != null)
...
Mark M.
this is massive overkill
Steve S.
ok
Mark M.
or, at least, it seems to be
Steve S.
sure. how so?
7:30 PM
Mark M.
https://github.com/commonsguy/cw-omnibus/tree/m...
in my case, I am showing a Toast, but you could update your TextView
you don't need a subclass of Handler, as postDelayed() exists on any View
Steve S.
i see
Mark M.
you don't need WeakReference, as you know when your TextView is going away and can call removeCallbacks() at that point
Steve S.
ok - you know because onStop() is called?
Mark M.
and postDelayed() (on View or Handler) is inexact, as you may be tying up the main application thread when the specific millisecond arises
correct, or onDestroy(), or whatever lifecycle method makes sense in your scenario
Steve S.
it doesn't have to be exact - an approximation is ok
Mark M.
OK, but you could just call System.currentTimeMillis() and calculate the difference between your original time and now, and get an accurate result
Steve S.
ok
this is great. i'll review your example and try to simplify things
Mark M.
there's material on it in the book
Steve S.
great, i'll review that too
on another topic, i had a follow-up question on the service we talked about last time
7:35 PM
Steve S.
i've tried to respond to the issue you raised
here's the new version of the class i showed you last time:
View paste (6 more lines) 
static private class ConnectThread extends Thread {
        final private BluetoothAdapter mmAdapter;

        ConnectThread(BluetoothAdapter a) {
            mmAdapter = a;
        }

        @Override
        public void run() {
            super.run();
            if (!mmAdapter.isEnabled()) {
                postStatus(Status.DISABLED);
                synchronized (LOCK) {
                    mConnectThread = new ConnectThread(mmAdapter);
                    mConnectThread.start();
...
Mark M.
you have a thread that is... starting another copy of the thread?
Steve S.
yes. so that's a question: is that ok?
Mark M.
it feels like it'll be an infinite loop
(ish, considering that it's not technically a loop)
Steve S.
it may be, but that's acceptable for our application
Mark M.
you start the first ConnectThread
Steve S.
i'll show you the code that starts things off:
View paste 
void start() {
        synchronized (LOCK) {
            if (mConnectThread != null)
                return;
            BluetoothAdapter a = BluetoothAdapter.getDefaultAdapter();
            if (a == null) {
                postError(Status.UNSUPPORTED);
                stopSelf();
            } else {
                mConnectThread = new ConnectThread(a);
                mConnectThread.start();
            }
        }
    }
Mark M.
that seems like it would still create thousands of threads a second, causing your device to catch on fire
start() is asynchronous, as that's the point of a thread
(rephrase: start() on a Thread is asynchronous)
Steve S.
right
Mark M.
so, here's what I see:
you call the start() method in your second paste
Steve S.
ok
Mark M.
it gets an adapter
it creates a ConnectThread and calls start() on it
7:40 PM
Mark M.
this then exits the LOCK block
Steve S.
right
Mark M.
meanwhile, ConnectThread invokes run()
run() sees that the adapter is not enabled
run calls postError() and creates/starts another ConnectThread
'
ConnectThread #2 invokes run(), sees that the adapter is not enabled, calls postError(), and creates/starts another ConnectThread
ConnectThread #3 invokes run(), sees that the adapter is not enabled, calls postError(), and creates/starts another ConnectThread
ConnectThread #4 invokes run(), sees that the adapter is not enabled, calls postError(), and creates/starts another ConnectThread
ad nauseum
Steve S.
that is correct
Mark M.
um, that's bad
Steve S.
i realize that this probably isn't a good idea in general
Mark M.
the Android/Linux thread scheduler isn't designed for that kind of abuse, and I'm not sure what you're gaining by this approach
Steve S.
what would be a better approach?
Mark M.
some sort of polling loop with a nice fat delay per pass or something
my assumption is that you want to find out as soon as the adapter is enabled
Steve S.
sure
Mark M.
and my further assumption is that the Bluetooth APIs don't offer a callback mechanism for that (as I haven't played with those APIs)
Steve S.
no callback as far as i know
Mark M.
in which case, having *a* thread monitor the adapter isn't necessarily a bad thing
but that's *a* thread, not a continuous sequence of rapidly-spawned threads
Steve S.
so would what i have be better if i introduced a delay?
ok
Mark M.
use a while loop
7:45 PM
Steve S.
ok
Mark M.
while not enabled, sleep for X milliseconds, for as big a value of X as you can reasonably stand
Steve S.
ok - you mean Thread#sleep()?
Mark M.
or SystemClock.sleep(), which saves you dealing with the InterruptedException
Steve S.
great
Mark M.
this sort of polling isn't ideal -- if there were a callback, use it
but, a simple check-and-sleep loop is way better than bludgeoning your CPU with a lead pipe
Steve S.
great. very helpful
i'll take another pass at this then
i have one last question, on CountDownTimer
Mark M.
if it has to do with what isTheFinalCountdown() actually does, I haven't tried it
Steve S.
i don't think i'm up to that version of Android yet, but I'll look for it
Mark M.
I really hope it plays the Europe song, but I doubt it :-)
7:50 PM
Mark M.
it's new to Android 8.0, yet another SDK Easter egg
Steve S.
is it in your booik?
*book
Mark M.
no, but I mentioned it in one of my "random musings" blog posts, pointing out the relevant GEICO commercial
Steve S.
i'll check it out!
Mark M.
but, that's apparently not your question
Steve S.
not yet...what i'm wondering is, to avoid memory leaks, do i need to create a static subclass of CountDownTimer, or is it ok to create an anonymous class?
Mark M.
beats me
I don't think I have ever used it
the JavaDocs shows an anonymous class
so, that's a positive sign that an anonymous class is OK
Steve S.
ok. i did look at the source code, and internally it uses a Handler class with postDelated(), which isn't static
Mark M.
hmmm
I tend to use other things (postDelayed() for UI stuff, SecheduledExecutorService for background stuff)
*presumably* their use of Handler is OK, as probably somebody would have pointed out a leak already
Steve S.
ok
Mark M.
and I'm not seeing any issues about leaks, other than this one: https://issuetracker.google.com/issues/37094374
7:55 PM
Mark M.
and that seems to be tied to something else
so, you're probably OK with an anonymous class, but I can't be sure
Steve S.
should i not be ok with that?
Mark M.
:: shrug ::
Steve S.
ok
Mark M.
like I said, I don't use it, but that's more that I like the alternatives that I cited better
Steve S.
ok
Mark M.
but if there are no issues, and the JavaDocs show an anonymous class, it is likely that an anonymous class is OK
"likely" is not "guaranteed"
Steve S.
sure
i suppose i could test it myself to see if there are any problems
no more questions today. i really appreciate all your help! i'll take another stab at the BluetoothService
have a great rest of the day!
Mark M.
you too!
Steve S.
has left the room
8:30 PM
Mark M.
turned off guest access

← Yesterday, October 4

 

Office Hours

People in this transcript

  • Mark Murphy
  • Steve S