> On Sept. 30, 2013, 4:22 a.m., Kelven Yang wrote:
> > Daren,
> >
> > when getGuru() is call in a non-initial phase, it may be involved with
> > multi-threaded context, I suggest to make it thread-safe
> >
> >
> > public HypervisorGuru getGuru(HypervisorType hypervisorType) {
> > 58
> > return _hvGurus.get(hypervisorType);
> > 60
> > HypervisorGuru result = _hvGurus.get(hypervisorType);
> > 61
> >
> > 62
> > if ( result == null ) {
> > 63
> > for ( HypervisorGuru guru : _hvGuruList ) {
> > 64
> > if ( guru.getHypervisorType() == hypervisorType ) {
> > 65
> > _hvGurus.put(hypervisorType, guru);
> > 66
> > result = guru;
> > 67
> > break;
> > 68
> > }
> > 69
> > }
> > 70
> > }
> > 71
> >
> > 72
> > return result;
> > 59
> > }
> > 73
> >
> > -Kelven
_hvGurus is a concurrenthashmap, so the map is thread safe. The overall action
will be thread safe. What can happen is that two threads in parallel can hit
the if ( result == null ) condition, and they will both traverse the list
looking for a match and both find a match and both update the _hvGurus map. So
in a race condition, you can have some wasted CPU cycles, but no real harm.
Would rather have that then always synchronize on this method.
- Darren
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14190/#review26468
-----------------------------------------------------------
On Sept. 18, 2013, 3:49 p.m., Darren Shepherd wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14190/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2013, 3:49 p.m.)
>
>
> Review request for cloudstack, Alex Huang and Kelven Yang.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Various classes are using member injection to inject extensible objects.
> Really those object should come from an AdapterList that is injected in.
> This patch switches the code to use setter injection that will later allow
> spring to inject an AdapterList or something similar to allow extensibility
>
>
> Diffs
> -----
>
> engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java
> 24f0795
>
> engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java
> 204b832
>
> plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java
> d4d73d1
> server/src/com/cloud/api/ApiServer.java 550626f
> server/src/com/cloud/configuration/ConfigurationManagerImpl.java 17ef6bf
> server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java 90273f7
> server/src/com/cloud/hypervisor/HypervisorGuruManagerImpl.java 4d1e1b5
> server/src/com/cloud/network/NetworkServiceImpl.java eb63fe0
> server/src/com/cloud/server/ManagementServerImpl.java c0a52f7
> server/src/com/cloud/storage/VolumeApiServiceImpl.java cc99589
> server/src/com/cloud/storage/secondary/SecondaryStorageManagerImpl.java
> d4463d9
> server/src/com/cloud/template/TemplateManagerImpl.java e11ac0d
>
> Diff: https://reviews.apache.org/r/14190/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Darren Shepherd
>
>
