While updating a customer project from MPS 2019.1 to MPS 2019.3 we came across a bug inside of MPS which ultimately ended up in us sending a pull request to the code base. In this post I would like to write about our experience how tracked it down. The final fix for the bug is a one line change:
After Alex, who was doing the migration, fixed some of the problems that prevented our generator tests from working he noticed strange behavior in some of our generators. The behavior we saw was: accessing property values in the generator failed. At first glance this looked like wrong generator priorities or a broken condition somewhere, which the concept instances where not transformed correctly and therefor accessing the properties yielded no value. It was quickly discovered that none of these was the case. The nodes in question weren’t created during the generation but where concept instances that were part of the input model created for the tests. Now things started to get a little strange. Looking at the transient models, MPSs way of debugging generators, the nodes where there with the correct structure, which was checked with the node explorer, and accessing the properties on them worked. The conclusion was: it must be a generator specific problem. Alex continued to debug the problem and noticed that it only affected a small amount of properties, 4 concepts were affected. After some time it was discovered that the all the properties and concepts containing them had the same id. Now we knew we are onto something.
Since MPS technically uses a hash map to store all the nodes and their properties, it needs to construct keys for accessing a property or child on a node. For properties MPS uses three identifiers to uniquely identify it: each language has it own unique id, each concept has its own id which is unique within the language and finally each property has its own id which unique within the concept. By building the key over those three parts MPS can globally address a property.
Having duplicate concept and property ids isn’t a problem since the key for looking them up includes the language id. The code for looking up a property is in MeteAdapterFactory:
@NotNull
public static SProperty getProperty(long uuidHigh, long uuidLow, long concept, long prop, String propName) {
return property(uuidHigh, uuidLow, concept, prop, () -> new PropertyBucket(uuidHigh, uuidLow, concept, prop, propName));
}
Alex figured out that changing the generator and accessing the property in the condition of the generator before accessing the same property in the rule consequence seemingly fixed the problem. We later discovered that it only moved the problem to another place. After more analyses it was found that it was related to the generator that generates the Java code for accessing properties. Since Alex wasn’t able to continue to work on the topic for the next two days he asked me to take it over. After going through all of his findings I continued to look into it. Alex already identified the generator responsible for generating the syntactic sugar from the smodel language into plain Java code. He provided me insights how to debug it with conditional break points as well.
The generator that was affected by the problem was quite large and debugging was not the straight forward. Lots of conditional breakpoints were required. Support for conditional breakpoints in IntelliJ is quite extensive but it was still hard to debug. The first thing I decided was to build a small project that reproduced the issue without the context of the customer project. I went ahead and created a project with two language where I copied a concept with properties from one language to the other. You can find the project here.
After reproducing the issue outside the customer project I started my work by taking a look at the generators involved and understanding them. The smodel language allows easy access to property on concept instance and is built into MPS. The language is generated down to Java code that interacts with the actual runtime classes. This means code like this:
public static int getXOnB(node<ConceptB> n) {
n.x;
}
is translated to
public static int getXOnB(SNode n) {
return SPropertyOperations.getInteger(n, PROPS.x$g$WK);
}
The generator responsible for this isreduce_PropertyDeclaration_SProperty that looks like this:
I won’t show the content of the property macros here since it’s all straight forward, the code extracts the three parts (language, concept and property id) from the referenced property. The first guess we had was that this code got somehow confused by the duplicated concept and property ids. A quick debugging session showed that this wasn’t the case, the ids were all calculated correctly. Since the ids were correct there must be something else happening after the ids were calculated. Looking at the generator template and resulting code in the Java files revealed a key difference: The template generates calls to the MeteAdapterFactory directly but in the resulting Java code these are stored in a static field. See the last part of this code: PROPS.x$g$WK.
public static int getXOnB(SNode n) {
return SPropertyOperations.getInteger(n, PROPS.x$g$WK);
}
PROPS in this case is an inner class that holds the constants:
private static final class PROPS {
static final SProperty x$g$WK = MetaAdapterFactory.getProperty(0xc96e212c969a47bfL, 0xbd4dca2da4aedeabL, 0x1b6616c72e608f91L, 0x1b6616c72e608f94L, "x");
}
In our case where we were accessing two different properties we would have expected the class to contain two members, one for each property. What we saw was just a single member that was used in both places. Since we saw in the debugger that both ids were calculated it seemed that the code responsible for merging the constants caused the problem. If you take a look at the template more closely you can see that the Java code is wrapped with extract(<> to single constant $PROP) where the $PROP is used to calculate a key for merging the values. It basically means the Java generator will extract a single constant with the expression in extract as an initializer and then replace all occurrences based on the key defined in $PROP. Lets take a look at how that key is calculated:
int hash = Objects.hash(node.propertyId, node.getConceptDeclaration().conceptId) & 0xFFFFFF;
node.name + "$" + new JavaFriendlyBase64().toString(hash);
Gotcha, the hash used to unify the values doesn’t include the language id. After this we decided to send a pull request to the code base here. In the end there was a little bit more to that we did in our pull request initially thought and Alexander from JetBrains added the code for covering that use case as well as part of his work when merging the change.
This blog post isn’t bashing MPS or making fun of the bug, these things can happen. What I wanted to illustrate is how finding such a small problem, an incorrect key for a dictionary, can be challenging in complex systems like MPS. I also wanted to share our experience how we approached the problem and how we finally fixed it.
I would like to thank Alex for his initial analyses and documenting the bug which otherwise would have taken me much longer to analyze. I would also like to thank JetBrains and Alexander at JetBrains who provided us a bugfix release quickly after we reported the problem and proposed the fix.
