一个典型的多线程问题

Posted on | In

一天，同事抛给我一个线上崩溃。堆栈如下（复现的代码）

1
2
3
4
5
6
java.lang.NullPointerException
    at java.util.Timer.scheduleImpl(Timer.java:561)
    at java.util.Timer.schedule(Timer.java:459)
    at run.yang.test.ThreadProblem.problematicMethod(ThreadProblem.java:21)
    at run.yang.test.TestTimerMultiThreadProblem$1.run(TestTimerMultiThreadProblem.java:25)
    at java.lang.Thread.run(Thread.java:841)

相应代码如下：

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
private static AtomicBoolean sIsExecuting = new AtomicBoolean(false);
private static Timer sTimer;

public static void problematicMethod(int ms) {
    if (sIsExecuting.get()) {
        return;
    }

    if (sTimer != null) {
        sTimer.cancel();
        sTimer.purge();
    }

    sTimer = new Timer();
    sTimer.schedule(new TimerTask() {
        @Override
        public void run() {
            sIsExecuting.set(true);
            doActualWork();
            sTimer = null;
            sIsExecuting.set(false);
        }
    }, ms >= 0 ? ms : 10000);
}

崩在 sTimer.purge()这一行。

显然是多线程导致的，查看调用problematicMethod的地方，果然是在不同线程里。首先AtomicBoolean的用法不对，起不到应有的作用，多个线程仍然可能进入这个方法体。第二，由于第一个问题，判断了sTimer != null并不能保证调用sTimer.cancel()及后的方法时sTimer仍然不为空。

这个问题属于有多线程的意识，但对AtomicBoolean不明所以，用的糊里糊涂。工作中之前也曾遇到过相同的问题，把原本的boolean直接替换成AtomicBoolean，只使用get, set却不使用compareAndSet。

修改后的代码：

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
private static final AtomicBoolean sIsExecuting = new AtomicBoolean(false);
private static final AtomicReference<Timer> sTimer = new AtomicReference<>();

public static void problematicMethod(int delayMillis) {
    if (sIsExecuting.get()) {
        return;
    }

    final Timer newTimer = new Timer("TimerName");
    newTimer.schedule(new TimerTask() {
        @Override
        public void run() {
            if (sIsExecuting.compareAndSet(false, true)) {
                doActualWork();

                final Timer thisTimer = sTimer.getAndSet(null);
                destroyTimer(thisTimer);
                sIsExecuting.set(false);
            }
        }
    }, delayMillis >= 0 ? delayMillis : 10000);

    final Timer oldTimer = sTimer.getAndSet(newTimer);
    destroyTimer(oldTimer);
}

private static void destroyTimer(Timer timer) {
    if (timer != null) {
        timer.cancel();
        timer.purge();
    }
}

顺便还修复了一个Timer未反初始化的问题。这个问题会造成Timer线程不退出。这也是非常容易忽视的问题，原代码在任务完成后直接将sTimer置空，而没有检查这时sTimer是否有值。

Java的Timer很不好用，添加的任务无法取消，只能把Timer自己cancel了，然后这个Timer就不能用了，想用只能再new一个。还是Android的Handler方便。