Bug: EventHandler is not thread safe

[NikColonel]

Hi community.

In iDempiere you can use any way handler on event in system. Example: Например: Callout, Rule, EventHandler and ModelValidator.

EventHandler - this part framework OSGI from package org.osgi.service.event. Used for handler events. At this moment used version 1.4.0 (maven).

In our project we use plugin Libero MFG. This plugin contain class for work with EventHandler: public class MFG_Validator extends AbstractEventHandler.

We finded serious error when high load in EventHandler. It turned out that the method AbstractEventHandler.getPO() calling next method org.osgi.service.event.Event.getProperty() which can return not correct data.

Class org.osgi.service.event.Event keeps information about event in object org.osgi.service.event.EventProperties which using storage private final Map<String, Object> properties.

Error occurs during processing much count parallels threads as EventHandler. Every call EventHandler call method org.osgi.service.event.Event.getProperty() which is not thread safe and can return data from parallel thread. More details about this error are described in the article Concurrency and HashMap.

This error occurs in the class MFG_Validator from plugin Libero MFG.

Error generate following known effects:

• EventHandler can use data from parallel thread;

• in some cases arise connect with DB with status “idle in transaction” due to next code in class PO.save():
  

trx = Trx.get(m_trxName, false);
if (trx == null)
{
// Using a trx that was previously closed or never opened
// Creating and starting the transaction right here, but please note
// that this is not a good practice
trx = Trx.get(m_trxName, true);
log.severe("(save) Transaction closed or never opened ("+m_trxName+") => starting now --> " + toString());
}

After that, the database contain connection with status "idle in transaction" and last query is "RELEASE SAVEPOINT".

As as solution this error, we rewrited class MFG_Validator for work with ModelValidator. Solution success work and system is stable work with high load.

Has anyone encountered such a problem? How did you solve it?

Best regards,

Palichmos.ru team.

p.s. sorry for my bad english

[Hiep Lq]

try to don't call setPo(getPO(event)); mean don't use instance variable po, it's share by multi thread

[Heng Sin Low]

Event is method variable in the EventAdmin.sendEvent call and since  EventAdmin.sendEvent is synchronous so there shouldn't be a thread safety issue here (there's no concurrent execution of multiple EventHandler for the same Event). The only exception I can think of is if a particular EventHandler implementation is creating a new thread to perform asynchronous/background processing.

--
You received this message because you are subscribed to the Google Groups "iDempiere" group.
To unsubscribe from this group and stop receiving emails from it, send an email to idempiere+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/idempiere/0f362fca-c234-4d15-903c-3f58c9d985cb%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[NikColonel]

Thanks for answer,

i will write test case that will show this error well. 

MFG_Validator (EventHandler) do not creating new thread to perform asynchronous/background processing.

пятница, 22 марта 2019 г., 9:52:55 UTC+4 пользователь hengsin написал:

[NikColonel]

Thanks, your right. Variable "PO" is sharing for multi thread.

Solve: move initialize variable "PO" to method doHandleEvent().

If use global variable in AbstractEventHandler then this variable will be access for multi thread (non thread save).

p.s. when i created test case then understanded this. Need more often do it)

пятница, 22 марта 2019 г., 6:02:50 UTC+4 пользователь Hiep Lq написал: