Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 358352

Summary: GrammarAccess unsufficiently synchronized
Product: [Modeling] TMF Reporter: Mark Christiaens <mark.g.j.christiaens>
Component: XtextAssignee: Project Inbox <tmf.xtext-inbox>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: lieven.lemiengre, mark.g.j.christiaens, sebastian.zarnekow, st.oehme, sven.efftinge
Version: 2.0.1   
Target Milestone: ---   
Hardware: PC   
OS: All   
Whiteboard: v2.7

Description Mark Christiaens CLA 2011-09-21 03:17:56 EDT 
Build Identifier: 20110615-0604

The generated grammar access class (in my case VhdlGrammarAccess) is a singleton.  It's used all over the place by multiple threads so needs good synchronization.  I'm worried about the lazy initialization that occurs.  For every grammar element, X, there is a pX field that is initialized to null.  That field is then lazily initialized when getXAccess() is invoked.  There is a serious risk that the pX field will be initialized multiple times and therefor that there are multiple XElements and ParserRules floating around corresponding to the same grammar entries.  That means in turn that identity comparisons can fail.  I haven't seen this actually occur in the wild but who knows. 

On a side note, I kind of doubt (but haven't checked this) that the lazy initialization does any good in this context.  I would imagine that all grammar elements will be needed in any serious use case.  Lazily initializing them seems to incur a continuous overhead that doesn't repay itself?


Reproducible: Always
Comment 1 Sven Efftinge CLA 2011-09-21 03:28:31 EDT 
+1 for eager and thread safe initialization

Btw.: It looks like org.eclipse.xtext.service.GrammarProvider.getGrammar(Object) was made thread safe on purpose but I doubt it is, because of the EPackage initialization. Comments?
Comment 2 Sven Efftinge CLA 2011-09-21 03:29:43 EDT 
(In reply to comment #1)
> Btw.: It looks like
> org.eclipse.xtext.service.GrammarProvider.getGrammar(Object) was made thread
> safe on purpose but I doubt it is, because of the EPackage initialization.
> Comments?

Forget it, I missed the inner if block.
Comment 3 Sebastian Zarnekow CLA 2014-08-07 08:13:19 EDT 
Pushed to review