Burke: One way to implement rule cache, which I do not find useful, and still provide a thread safe interface is by keeping reference count on the rule token / handle. From where I come, it’s important that OpenMRS as a framework design around multithreaded / multitasking systems seriously. In the same vein, I strongly agree and support that it is perfectly legal to have instance variables in a rule class (translated or otherwise) as a design decision. I do not see much usefulness for rule cache because I think correct rule cache management is more expensive then the little impact it might have on the memory footprint. I hope you will consider these design issues in fixing this and future versions. Vibha
From: [email protected] [mailto:[email protected]] On Behalf Of Burke Mamlin Sent: Thursday, October 13, 2011 12:50 PM To: [email protected] Subject: Re: [OPENMRS-DEV] Major Logic Thread Safety Issue I'm not sure it's fair to expect rule authors to avoid any instance variables. We either need to make new instances or – if that's a memory hog – we could avoid needing to create a new instance every time by introducing String getState() and void setState(String) methods to the Rule interface. -Burke On Thu, Oct 13, 2011 at 6:10 PM, Ben Wolfe <[email protected]<mailto:[email protected]>> wrote: I don't know of the implementation or reasoning here, but there is a lot less memory used if its not instantiating new objects each time. I think we should stick with that approach. We should either say that class level variables are not allowed (like on controllers and servlets) or we say they are allowed but all variables get cleared before a rule is used. Tammy, what was the fix in 1.5 for this? Is there a ticket? Was there a regression test for it originally? Ben On Thu, Oct 13, 2011 at 5:46 PM, Tammy Dugan <[email protected]<mailto:[email protected]>> wrote: It also corrupted 3 days worth of patient data across 4 clinics so needless to say we were pretty unhappy that a problem we fixed in 1.5 came back in 1.7 and corrupted patient data. Tammy On 10/13/2011 11:27 AM, McKee, Steven Jay wrote: Darius, CHICA encountered a major issue with Logic yesterday. We had information get crossed between multiple patients, and we tracked it down to a thread safety issue in Logic. Currently the Token Service’s “getRule(String)” method (in OpenMRS 1.7) retrieves a rule from a rule provider and then caches it. The next time the rule is asked for, it gets pulled directly from the cache. Our Java rules translated by the Arden translator had class level variables that were getting changed by concurrent threads, and we were seeing incorrect data for patients. In OpenMRS 1.5 the logic service would cache the rule, but it would return a new instantiation of the rule class so it was thread safe. Now in 1.7 + the thread safety no longer exists because the same instance of the rule is directly returned from the cache. This is a very high priority issue that needs to be resolved immediately. Thanks, Steve McKee Children’s Health Services Research Indiana University School of Medicine Phone: 317-278-9660<tel:317-278-9660> Email: [email protected]<mailto:[email protected]> ________________________________ Click here to unsubscribe<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l> from OpenMRS Developers' mailing list -- Tammy Dugan CHIRDL Technical Lead Children's Health Services Research IU School of Medicine ________________________________ Click here to unsubscribe<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l> from OpenMRS Developers' mailing list ________________________________ Click here to unsubscribe<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l> from OpenMRS Developers' mailing list ________________________________ Click here to unsubscribe<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l> from OpenMRS Developers' mailing list
