<p dir="auto"><strong><a href="https://jira.spring.io/secure/ViewProfile.jspa?name=andersk" rel="nofollow">Anders Kobberup</a></strong> opened <strong><a href="https://jira.spring.io/browse/SPR-9263?redirect=false" rel="nofollow">SPR-9263</a></strong> and commented</p>
<p dir="auto">We are having a rising amount of issues where the symptom is that random properties that have been autowired into our beans (via <code class="notranslate">@autowired</code> setter methods) turns our null, without any exceptions during start up.<br>
This means that we have no way of knowing whether our production setup is actually functioning as expected, or if it will encounter nullpointers as the diferent parts of the platform is used. Also the only way to "fix" any such nullpointer is to restart the application, which obviously is not good.</p>
<p dir="auto">I have traced the problem down to what i believe is a concurrency flaw in checkPropertySkipping in InjectionMetadata.InjectedElement.</p>
<div class="snippet-clipboard-content notranslate position-relative overflow-auto" data-snippet-clipboard-copy-content="172: protected boolean checkPropertySkipping(PropertyValues pvs) {
173: 	if (this.skip == null) {
174: 		if (pvs != null) {
175: 			synchronized (pvs) {
176: 				if (this.skip == null) {
177: 					if (this.pd != null) {
178: 						if (pvs.contains(this.pd.getName())) {
179: 							// Explicit value provided as part of the bean definition.
180: 							this.skip = true;
181: 							return true;
182: 						}
183: 						else if (pvs instanceof MutablePropertyValues) {
184: 							((MutablePropertyValues) pvs).registerProcessedProperty(this.pd.getName());
185: 						}
186: 					}
187: 				}
188: 			}
189: 		}
190: 		this.skip = false;
191: 	}
192: 	return this.skip;
193: }"><pre class="notranslate"><code class="notranslate">172: protected boolean checkPropertySkipping(PropertyValues pvs) {
173: 	if (this.skip == null) {
174: 		if (pvs != null) {
175: 			synchronized (pvs) {
176: 				if (this.skip == null) {
177: 					if (this.pd != null) {
178: 						if (pvs.contains(this.pd.getName())) {
179: 							// Explicit value provided as part of the bean definition.
180: 							this.skip = true;
181: 							return true;
182: 						}
183: 						else if (pvs instanceof MutablePropertyValues) {
184: 							((MutablePropertyValues) pvs).registerProcessedProperty(this.pd.getName());
185: 						}
186: 					}
187: 				}
188: 			}
189: 		}
190: 		this.skip = false;
191: 	}
192: 	return this.skip;
193: }
</code></pre></div>
<p dir="auto">Consider the scenario where at least two threads are creating an instance of a given bean, which have autowired properties. Both of these threads will enter the 'Inject(...)' method, and from here go to checkPropertySkipping (the argument for both threads are the same PropertyValues object).</p>
<p dir="auto">Thread A will enter the synchronized block, while Thread B will be halted before this block.<br>
Thread A will proceed to the inside of the synchronized block, and enter the 'else if' block, and register the propertyname in the PropertyValues object as processed (line 184).<br>
As soon as Thread A exits the synchronized block, the java scheduler halts Thread A and resumes Thread B which will enter the synch block and, as Thread A has been halted and thus not having set 'this.skip = false' (line 190), the 'if(this.skip == null)' statement (line 176) will return true.<br>
After this, thread A may continue running (the object that it is creating will be created correctly).</p>
<p dir="auto">Thread B will evaluate the 'if (pvs.contains(this.pd.getName()))' statement, which will return true, as Thread A has registered the property name as processed, and as a result Thread B will set 'this.skip = true' (line 180) and return.</p>
<p dir="auto">As 'skip' has been set to true on this InjectedElement, every later invocation of checkPropertySkipping after this will return true, and thus every later object that is instantiated will not have this dependency injected.</p>
<p dir="auto">The obvious solution is to ensure that 'this.skip = false' is also set within the synchronized block (add a line containing 'this.skip = false;' after line 187), thus ensuring that any threads that are awaiting the monitor will evaluate the if statement at line  176, to false and thus not setting 'this.skip = true'.</p>
<p dir="auto">In order to see if this is more than just a theoretical posibility that multiple threads will enter this code, i have created a small test class that can simulate this method being invoked by multiple threads. The test i have run, show that it will happen.</p>
<div class="snippet-clipboard-content notranslate position-relative overflow-auto" data-snippet-clipboard-copy-content="import java.util.ArrayList;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;

public class SynchTest {

	private final Object mutex = new Object();
	private Boolean finished;
	private AtomicInteger numberOfCompletions = new AtomicInteger( 0 );
	private int testNumber;

	public SynchTest( int TestNumber ) {
		this.testNumber = TestNumber;
	}

	public Runn getRunn() {
		return new Runn();
	}

	void gotThrough() {
		final int nCount = numberOfCompletions.incrementAndGet();
		if ( nCount &gt; 1 ) {
			//woops - this is not good!!
			System.out.println( &quot;In test &quot; + testNumber + &quot;, &quot; + nCount + &quot; got into the synch block!!&quot; );
		}
	}

	private class Runn implements Callable&lt;Object&gt; {

		@Override
		public Object call() throws Exception {
			if ( finished == null ) {
					synchronized ( mutex ) {
						if ( finished == null ) {
							gotThrough();
						}
					}
				finished = false;
			}
			return null;
		}
	}

	public static void main( String[] args ) throws InterruptedException {
		ExecutorService cThreadPool = Executors.newFixedThreadPool( 5 );
		ArrayList&lt;Runn&gt; cRunnables = new ArrayList&lt;&gt;();
		for ( int i = 0 ; i &lt; 1000 ; i++ ) {
			SynchTest cTest = new SynchTest( i );
			for ( int j = 0 ; j &lt; 5 ; j++ ) {
				cRunnables.add( cTest.getRunn() );
			}

		}
		System.out.println( &quot;Invoking &quot;+cRunnables.size()+&quot; tasks&quot; );
		cThreadPool.invokeAll( cRunnables );
		cThreadPool.shutdown();
		cThreadPool.awaitTermination( 5, TimeUnit.DAYS );
	}
}"><pre class="notranslate"><code class="notranslate">import java.util.ArrayList;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;

public class SynchTest {

	private final Object mutex = new Object();
	private Boolean finished;
	private AtomicInteger numberOfCompletions = new AtomicInteger( 0 );
	private int testNumber;

	public SynchTest( int TestNumber ) {
		this.testNumber = TestNumber;
	}

	public Runn getRunn() {
		return new Runn();
	}

	void gotThrough() {
		final int nCount = numberOfCompletions.incrementAndGet();
		if ( nCount &gt; 1 ) {
			//woops - this is not good!!
			System.out.println( "In test " + testNumber + ", " + nCount + " got into the synch block!!" );
		}
	}

	private class Runn implements Callable&lt;Object&gt; {

		@Override
		public Object call() throws Exception {
			if ( finished == null ) {
					synchronized ( mutex ) {
						if ( finished == null ) {
							gotThrough();
						}
					}
				finished = false;
			}
			return null;
		}
	}

	public static void main( String[] args ) throws InterruptedException {
		ExecutorService cThreadPool = Executors.newFixedThreadPool( 5 );
		ArrayList&lt;Runn&gt; cRunnables = new ArrayList&lt;&gt;();
		for ( int i = 0 ; i &lt; 1000 ; i++ ) {
			SynchTest cTest = new SynchTest( i );
			for ( int j = 0 ; j &lt; 5 ; j++ ) {
				cRunnables.add( cTest.getRunn() );
			}

		}
		System.out.println( "Invoking "+cRunnables.size()+" tasks" );
		cThreadPool.invokeAll( cRunnables );
		cThreadPool.shutdown();
		cThreadPool.awaitTermination( 5, TimeUnit.DAYS );
	}
}
</code></pre></div>
<p dir="auto">Heres the result of just one run:</p>
<p dir="auto">Invoking 5000 tasks<br>
In test 695, 2 got into the synch block!!<br>
In test 765, 2 got into the synch block!!<br>
In test 824, 2 got into the synch block!!<br>
In test 872, 2 got into the synch block!!<br>
In test 981, 2 got into the synch block!!</p>
<hr>
<p dir="auto"><strong>Affects:</strong> 3.1.1</p>
<p dir="auto"><strong>Issue Links:</strong></p>
<ul dir="auto">
<li><a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="398153507" data-permission-text="Title is private" data-url="https://github.com/spring-projects/spring-framework/issues/14439" data-hovercard-type="issue" data-hovercard-url="/spring-projects/spring-framework/issues/14439/hovercard" href="https://github.com/spring-projects/spring-framework/issues/14439">#14439</a> Autowired properties can remain unset during concurrent instantiation of prototype-beans (<em><strong>"duplicates"</strong></em>)</li>
<li><a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="398118207" data-permission-text="Title is private" data-url="https://github.com/spring-projects/spring-framework/issues/13894" data-hovercard-type="issue" data-hovercard-url="/spring-projects/spring-framework/issues/13894/hovercard" href="https://github.com/spring-projects/spring-framework/issues/13894">#13894</a> Concurrent retrieval of prototype-scoped beans may result in null <code class="notranslate">@Autowired</code> fields</li>
</ul>
<p dir="auto"><strong><a href="https://jira.spring.io/secure/ViewProfile.jspa?name=olamy" rel="nofollow">Olivier Lamy</a></strong> opened <strong><a href="https://jira.spring.io/browse/SPR-5385?redirect=false" rel="nofollow">SPR-5385</a></strong> and commented</p>
<p dir="auto">All artifactIds has changed in 3.0M1.<br>
It will be a REAL NIGTHMARE for maven users when upgrading due to all artifactIds change !!<br>
This will break a lot of builds for people who wants to upgrade !!<br>
Please any chance to revert to this non backward compatible change ????</p>
<hr>
<p dir="auto">Olivier<br>
Maven PMC</p>
<hr>
<p dir="auto"><strong>Affects:</strong> 3.0 M1</p>
<p dir="auto"><strong>Issue Links:</strong></p>
<ul dir="auto">
<li><a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="398091977" data-permission-text="Title is private" data-url="https://github.com/spring-projects/spring-framework/issues/9974" data-hovercard-type="issue" data-hovercard-url="/spring-projects/spring-framework/issues/9974/hovercard" href="https://github.com/spring-projects/spring-framework/issues/9974">#9974</a> Fix Maven POMs (<em><strong>"depends on"</strong></em>)</li>
</ul>
<p dir="auto">12 votes, 12 watchers</p>