OpenJDK / graal / graal-jvmci-8
changeset 9720:d30cc6543973
fix concurrency issue in NodeClass.get
author | Lukas Stadler <lukas.stadler@jku.at> |
---|---|
date | Wed, 15 May 2013 17:29:30 +0200 |
parents | f6b1694360ec |
children | 65452ead4b71 b88f69f80681 |
files | graal/com.oracle.graal.graph/src/com/oracle/graal/graph/NodeClass.java |
diffstat | 1 files changed, 17 insertions(+), 8 deletions(-) [+] |
line wrap: on
line diff
--- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/NodeClass.java Wed May 15 14:56:52 2013 +0200 +++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/NodeClass.java Wed May 15 17:29:30 2013 +0200 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -37,13 +37,22 @@ return clazz; } - // We can have a race of multiple threads creating the LIRInstructionClass at the same time. - // However, only one will be put into the map, and this is the one returned by all threads. - clazz = new NodeClass(c); - NodeClass oldClazz = (NodeClass) allClasses.putIfAbsent(c, clazz); - if (oldClazz != null) { - return oldClazz; - } else { + /* + * Using putIfAbsent doesn't work here, because the creation of NodeClass needs to be + * serialized. (the NodeClass constructor looks at allClasses, and it also uses the static + * field nextIterableId) + * + * The fact that ConcurrentHashMap.put and .get are used should make the double-checked + * locking idiom work, since it internally uses volatile. + */ + + synchronized (allClasses) { + clazz = (NodeClass) allClasses.get(c); + if (clazz == null) { + clazz = new NodeClass(c); + NodeClass oldClass = (NodeClass) allClasses.putIfAbsent(c, clazz); + assert oldClass == null; + } return clazz; } }