Background with serial not working

[himamis]

The Background annotation with serial does not work always as expected, in the case, when it's called from another background thread.

It is expected that methods annotated with the @background(serial = X) annotation (provided X is the same for all of them) will run in the order that they were called.

Example

package org.androidannotations.sample;

import org.androidannotations.annotations.Background;
import org.androidannotations.annotations.Click;
import org.androidannotations.annotations.EActivity;

import android.app.Activity;
import android.util.Log;

@EActivity(R.layout.my_activity)
public class MyActivity extends Activity {

    @Click
    void myButtonClicked() {
        bgTaskCallingOthers();
    }

    @Background
    void bgTaskCallingOthers() {
        // we expect that first the long finishes, then the short.
        backgroundSerialLong();
        backgroundSerialShort();
    }

    @Background(serial = "same")
    void backgroundSerialShort() {
        Log.w("MyActivity", "Background serial short finished");
    }

    @Background(serial = "same")
    void backgroundSerialLong() {
        try {
            Thread.sleep(5000);
        } catch (InterruptedException ex) {
            // ignore
        } finally {
            Log.w("MyActivity", "Background serial long finished");
        }
    }
}

In the above example, we expect that first the backgroundSerialLong method then the second backgroundSerialShort method prints it's log message.

But sometimes it's the other way around, resulting in the following log:

05-19 08:24:49.480 15669-15839/org.androidannotations.sample W/MyActivity: Background serial short finished
05-19 08:24:54.480 15669-15765/org.androidannotations.sample W/MyActivity: Background serial long finished

My first thought, is that sometimes the executor starts to run the task immediately, which in it's first line calls managed.getAndSet(true), so in BackgroundExecutor.execute(Task task), in the second if, the task is not added to the TASKS field.

Because of this, the second background task, doesn't know about the first one.

UPDATE
I've tested this by calling the aforementioned two methods from the myButtonClicked method (which runs on the main thread) and the same thing happens. Sometimes it works, sometimes it doesn't. Around 2 out of 5 times it fails.

[himamis]

@rom1v I see, that you have been working on the BackgroundExecutor class some time ago. Can you please check?

[rom1v]

Hi,

Thank you for the notification.

My first thought, is that sometimes the executor starts to run the task immediately, which in it's first line calls managed.getAndSet(true), so in BackgroundExecutor.execute(Task task), in the second if, the task is not added to the TASKS field.

I just read the current version of the BackgroundExecutor (not tested), I agree with your analysis.

If two tasks are executed in the wrong order, then they have both been submitted to the executor. As you said, the reason is that hasSerialRunning(task.serial) returns false the second time, because the tasks has not been added to TASKS.

IMO this has been introduced by 99a5170 (#1689) (cc @guitcastro).

However, org.androidannotations.test.ThreadActivityTest.serializedBackgroundTasks() should have catched the problem.

Could you improve the test (or add another one) to make it fail like in your case, please?

[WonderCsabo]

Hey guys,

Thanks for managing this issue. Sorry for the inconvenience caused by this bug.
The BackgroundExecutor is one of the most complex classes in AA, due to heavy concurrent
code. I try to review the PRs carefully, but i also make mistakes in this area. 😢
I would be very happy if you could provide a fix.

[rom1v]

I would be very happy if you could provide a fix.

I got some time today to work on the problem: #1803.

@himamis @guitcastro, could you check if it works for you, please?

[guitcastro]

Sorry guys, I am complete out time right now.
Also sorry for break the serial executor.

[guitcastro]

ps: My wife just had a kid, that's why I didn't answer the previous mention and is also why I am out of time.

=)

[rom1v]

Also sorry for break the serial executor.

Thanks for having spotted the issue with a synchronous executor.

That's unfortunate that the problem was not catched by unit tests. I tried to write a unit test to reproduce it, but I didn't manage to observe a wrong behavior due to the race condition from there. I only reproduced it on Android from your sample.

My wife just had a kid

Concrats! And good luck 😛 😫

[LouisCAD]

Hi everyone!

I have a really similar issue I originally posted as a duplicate in #1806
I don't have good news, but I have something that may help you! The sample you'll find below may help for reproducing the issue (happens all the time) and for writing unit tests!

I am using the @Background annotation in an app to perform network requests. I added a serial in the annotation parameters, but my backend developer and me discovered that multiple requests are made at once.

I then discovered that this part of the doc:

If you want execute some background methods SEQUENTIALLY, you should simply use serial() field. All task with the same serial key will be executed sequentially.

is not respected. Any running @Backround annotated method will make them run in PARALLEL.

I made sample, which didn't reproduce the issue at first. I then added another background job, and I saw tasks with same serial starting concurrently from the logs.

Here's the sample

You can look at MainActivity.class where there's a dummy Background job, which disturbs the one in MyBean.class which is started as soon as you press the pink Floating Action Button.

The implementation is absolutely straightforward, and the short classes I quoted are the only classes that matters.

Expected output:

Hello from 1282!
Goodbye from 1282!
Hello from 1279!
Goodbye from 1279!
Hello from 1279!
Goodbye from 1279!
Hello from 1279!
Goodbye from 1279!
Hello from 1278!
Goodbye from 1278!
Hello from 1281!
Goodbye from 1281!
Hello from 1277!
Goodbye from 1277!
Hello from 1274!
Goodbye from 1274!
Hello from 1283!
Goodbye from 1283!
Hello from 1280!
Goodbye from 1280!

Actual output:

Hello from 1314!
Hello from 1307!
Hello from 1315!
Hello from 1311!
Hello from 1316!
Hello from 1313!
Hello from 1317!
Hello from 1312!
Goodbye from 1314!
Goodbye from 1307!
Goodbye from 1315!
Goodbye from 1316!
Goodbye from 1313!
Goodbye from 1317!
Goodbye from 1311!
Hello from 1307!
Hello from 1315!
Goodbye from 1312!
Goodbye from 1315!
Goodbye from 1307!