Thread safety issue with XResourceLoader

[eric-kampf]

I have experienced a threading issue with org.robolectric.res.XResourceLoader. Observe the initialize method:

 void initialize() {
        if (isInitialized) return; 
        doInitialize();
        isInitialized = true;
        makeImmutable();
 }

Note that the "isInitialized" flag, which appears to prevent multiple initializations, does not provide full protection. It is possible that 2 or more threads could call the "doInitialize()" method. In fact, I have seen that in my Android application. When this occurs, bad things happen (concurrent modification of data structures) that break the Robolectric test.

My recommendation is that the "initialize" method of this class be synchronized.

[JurgenCruz]

I was under the impression that robolectric runs on a single thread.

[eric-kampf]

Robolectric runs a single thread however the application that it launches is free to spawn its own threads. Robolectric initializes its resource classes on-demand. That is, the first attempt, by the app, to read a resource, will trigger the initialization code above. That is what happened in my case. 2 threads were both triggering this initialization thus causing a race condition that lead to frequent failures.

So given that the class above has restrictions on how it can be used, it would be beneficial to enforce this in code (i.e. synchronize the method) rather than assuming that it will only be used in a certain manner.

[eric-kampf]

@erd - It is not clear to me why this was closed. I did not see any comments on the matter. The diffs in the commit appeared to be in an entirely unrelated area. I'm new to GitHub, so perhaps I am missing something. Could you please elaborate on why the issue was closed and if a fix was implemented?

[JakeWharton]

It was merged manually. When pushed to GitHub the PR was automatically
closed.
On Feb 28, 2014 8:50 AM, "eric-kampf" notifications@github.com wrote:

@erd https://github.com/erd - It is not clear to me why this was
closed. I did not see any comments on the matter. The diffs in the commit
appeared to be in an entirely unrelated area. I'm new to GitHub, so perhaps
I am missing something. Could you please elaborate on why the issue was
closed and if a fix was implemented?

Reply to this email directly or view it on GitHubhttps://github.com//issues/968#issuecomment-36370307
.

[eric-kampf]

@JakeWharton - So are you saying that a fix was committed? I checked what I believe is the latest version of the file in question, org.robolectric.res.XResourceLoader.java, and I see no changes. The threading issue is still present. Could you please elaborate.

[JakeWharton]

The change should be here:
72fb90a
On Feb 28, 2014 8:56 AM, "eric-kampf" notifications@github.com wrote:

@JakeWharton https://github.com/JakeWharton - So are you saying that a
fix was committed? I checked what I believe is the latest version of the
file in question, org.robolectric.res.XResourceLoader.java, and I see no
changes. The threading issue is still present. Could you please elaborate.

Reply to this email directly or view it on GitHubhttps://github.com//issues/968#issuecomment-36371029
.

[eric-kampf]

@JakeWharton - The changes in the provided link are completely unrelated to the issue I entered. So I ask again: Did someone actually fix the issue I entered: yes or no?

[JakeWharton]

Dunno. I've been replying from my phone. If that SHA doesn't match your
code then the answer to that would seem clear.
On Feb 28, 2014 9:11 AM, "eric-kampf" notifications@github.com wrote:

@JakeWharton https://github.com/JakeWharton - The changes in the
provided link are completely unrelated to the issue I entered. So I ask
again: Did someone actually fix the issue I entered: yes or no?

Reply to this email directly or view it on GitHubhttps://github.com//issues/968#issuecomment-36372505
.

[kingargyle]

I think this needs to be reopened, as @erd accidently put the wrong issue number on that commit. That commit should have referrenced issue #986

[eric-kampf]

@erd et al. I do not have authorization to re-open this. Could someone please take care of that for me? Thanks.

[erd]

@eric-kampf Yes, I typoed the issue number in a different commit. My apologies.

[eric-kampf]

Any plans to fix this? It's a very obvious bug with a simple fix.

[pohh]

I believe #1083 could be related to this. I'm able to consistently repro something similiar in 2 environments:

• osx 10.10.1, Java(TM) SE Runtime Environment (build 1.8.0_25-b17), build tools 21.1.2
• ubuntu 12 .04, Java(TM) SE Runtime Environment (build 1.7.0_72-b14), build tools 21.1.2

I tried to get a 3.0-SNAPSHOT build running locally to implement the fix described by @eric-kampf but I am running into some issues setting up my local maven repo to resolve dependencies.

OSX environment seems to run tests (edit: about 50% failure rate), ubuntu misses 90% of the time.

[jponsvega]

I'm trying to work around this issue, besides modifying the code to synchronize that piece, has anyone found a way to resolve it? I'm seeing the issue appear intermittently in OSX, and recurrently on a Ubuntu environment.

[xian]

Resource loading now happens before the test starts, so this issue is resolved.