Openbravo Issue Tracking System - Retail Modules
View Issue Details
0035897Retail ModulesStoreServerpublic2017-05-02 18:082017-05-26 08:07
AugustoMauch 
mtaal 
normalmajorhave not tried
closedfixed 
5
 
 
AugustoMauch
No
0035897: Prevent doing a commit inside an event handler (InvalidOrganizationChangeEventHandler)
Transactions must not be committed inside an event handler, or an error like this one [1] can occur. The InvalidOrganizationChangeEventHandler invokes the MobileServerProvider.getMobileServersForOrganization method, which can end up committing a transaction if some internal structure is not initialized. That method can also be called with a boolean parameter that determines if the transaction should be commited, that version should be used instead of the current one.

[1] cdb72c84 219658 [http-bio-8080-exec-5] ERROR org.openbravo.retail.copystore.process.CopyStoreProcess -
javax.enterprise.event.ObserverException
    at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
    at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
    at java.lang.Class.newInstance(Class.java:442)
    at org.jboss.weld.util.reflection.SecureReflections$16.work(SecureReflections.java:344)
    at org.jboss.weld.util.reflection.SecureReflectionAccess.run(SecureReflectionAccess.java:52)
    at org.jboss.weld.util.reflection.SecureReflectionAccess.runAsInstantiation(SecureReflectionAccess.java:173)
    at org.jboss.weld.util.reflection.SecureReflections.newInstance(SecureReflections.java:341)
    at org.jboss.weld.injection.Exceptions.rethrowException(Exceptions.java:33)
    at org.jboss.weld.injection.Exceptions.rethrowException(Exceptions.java:73)
    at org.jboss.weld.injection.MethodInjectionPoint.invokeOnInstanceWithSpecialValue(MethodInjectionPoint.java:162)
    at org.jboss.weld.event.ObserverMethodImpl.sendEvent(ObserverMethodImpl.java:245)
    at org.jboss.weld.event.ObserverMethodImpl.sendEvent(ObserverMethodImpl.java:233)
    at org.jboss.weld.event.ObserverMethodImpl.notify(ObserverMethodImpl.java:213)
    at org.jboss.weld.event.ObserverNotifier.notifyObserver(ObserverNotifier.java:117)
    at org.jboss.weld.event.ObserverNotifier.notifyObservers(ObserverNotifier.java:85)
    at org.jboss.weld.event.ObserverNotifier.fireEvent(ObserverNotifier.java:80)
    at org.jboss.weld.event.EventImpl.fire(EventImpl.java:69)
    at org.openbravo.client.kernel.event.PersistenceEventOBInterceptor.sendUpdateEvent(PersistenceEventOBInterceptor.java:106)
    at org.openbravo.client.kernel.event.PersistenceEventOBInterceptor.onFlushDirty(PersistenceEventOBInterceptor.java:75)
    at org.openbravo.client.kernel.event.PersistenceEventOBInterceptor$Proxy$_$$_WeldClientProxy.onFlushDirty(PersistenceEventOBInterceptor$Proxy$_$$_WeldClientProxy.java)
    at org.openbravo.dal.core.OBInterceptor.onFlushDirty(OBInterceptor.java:194)
    at org.hibernate.event.def.DefaultFlushEntityEventListener.invokeInterceptor(DefaultFlushEntityEventListener.java:372)
    at org.hibernate.event.def.DefaultFlushEntityEventListener.handleInterception(DefaultFlushEntityEventListener.java:349)
    at org.hibernate.event.def.DefaultFlushEntityEventListener.scheduleUpdate(DefaultFlushEntityEventListener.java:287)
    at org.hibernate.event.def.DefaultFlushEntityEventListener.onFlushEntity(DefaultFlushEntityEventListener.java:155)
    at org.hibernate.event.def.AbstractFlushingEventListener.flushEntities(AbstractFlushingEventListener.java:219)
    at org.hibernate.event.def.AbstractFlushingEventListener.flushEverythingToExecutions(AbstractFlushingEventListener.java:99)
    at org.hibernate.event.def.DefaultFlushEventListener.onFlush(DefaultFlushEventListener.java:50)
    at org.hibernate.impl.SessionImpl.flush(SessionImpl.java:1216)
    at org.openbravo.dal.service.OBDal.flush(OBDal.java:260)
    at org.openbravo.dal.core.SessionHandler.flushRemainingChanges(SessionHandler.java:680)
    at org.openbravo.dal.core.SessionHandler.commitAndClose(SessionHandler.java:592)
    at org.openbravo.dal.service.OBDal.commitAndClose(OBDal.java:223)
    at org.openbravo.replication.symmetricds.util.MobileServerProvider.initializeDataCachedByOrganization(MobileServerProvider.java:231)
    at org.openbravo.replication.symmetricds.util.MobileServerProvider.getMobileServersForOrganization(MobileServerProvider.java:179)
    at org.openbravo.replication.symmetricds.util.MobileServerProvider.getMobileServersForOrganization(MobileServerProvider.java:169)
    at org.openbravo.retail.storeserver.synchronization.eventhandler.InvalidOrganizationChangeEventHandler.anyStoreServerHasNowAccessToRecord(InvalidOrganizationChangeEventHandler.java:144)
I have not been able to reproduce this one, the log came from some BUT tests.
No tags attached.
depends on backport 0035999RR17Q2.1 closed mtaal Prevent doing a commit inside an event handler (InvalidOrganizationChangeEventHandler) 
Issue History
2017-05-02 18:08AugustoMauchNew Issue
2017-05-02 18:08AugustoMauchAssigned To => AugustoMauch
2017-05-02 18:08AugustoMauchTriggers an Emergency Pack => No
2017-05-02 18:08AugustoMauchSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=15091#r15091
2017-05-02 18:37AugustoMauchReview Assigned To => mtaal
2017-05-03 09:59hgbotCheckin
2017-05-03 09:59hgbotNote Added: 0096328
2017-05-03 09:59hgbotStatusnew => resolved
2017-05-03 09:59hgbotResolutionopen => fixed
2017-05-03 09:59hgbotFixed in SCM revision => http://code.openbravo.com/erp/pmods/org.openbravo.retail.storeserver.synchronization/rev/9b51903469413969a79ddf223d9de412f30ad555 [^]
2017-05-07 21:38mtaalNote Added: 0096436
2017-05-07 21:38mtaalStatusresolved => new
2017-05-07 21:38mtaalResolutionfixed => open
2017-05-08 11:52mtaalAssigned ToAugustoMauch => mtaal
2017-05-15 16:31mtaalStatusnew => scheduled
2017-05-19 11:13hgbotCheckin
2017-05-19 11:13hgbotNote Added: 0096675
2017-05-19 11:13hgbotStatusscheduled => resolved
2017-05-19 11:13hgbotResolutionopen => fixed
2017-05-19 11:13hgbotFixed in SCM revisionhttp://code.openbravo.com/erp/pmods/org.openbravo.retail.storeserver.synchronization/rev/9b51903469413969a79ddf223d9de412f30ad555 [^] => http://code.openbravo.com/erp/pmods/org.openbravo.replication.symmetricds/rev/adfb82ccf42286d731d4d7e68b766932f75b7d3e [^]
2017-05-19 11:15mtaalReview Assigned Tomtaal => AugustoMauch
2017-05-19 11:15hgbotCheckin
2017-05-19 11:15hgbotNote Added: 0096676
2017-05-22 15:36AugustoMauchNote Added: 0096713
2017-05-22 15:36AugustoMauchStatusresolved => closed
2017-05-26 08:06hgbotCheckin
2017-05-26 08:06hgbotNote Added: 0096825
2017-05-26 08:07hgbotCheckin
2017-05-26 08:07hgbotNote Added: 0096826

Notes
(0096328)
hgbot   
2017-05-03 09:59   
Repository: erp/pmods/org.openbravo.retail.storeserver.synchronization
Changeset: 9b51903469413969a79ddf223d9de412f30ad555
Author: Augusto Mauch <augusto.mauch <at> openbravo.com>
Date: Tue May 02 18:37:06 2017 +0200
URL: http://code.openbravo.com/erp/pmods/org.openbravo.retail.storeserver.synchronization/rev/9b51903469413969a79ddf223d9de412f30ad555 [^]

Fixes issue 35897: Prevents doing a commit inside an eventhandler

The eventhandler InvalidOrganizationChangeEventHandler now uses the method MobileServerProvider.getMobileServersForOrganization(String organizationId, boolean doCommit) instead of MobileServerProvider.getMobileServersForOrganization(String organizationId), which commits the transaction if some internal structure is not initialized.

---
M src/org/openbravo/retail/storeserver/synchronization/eventhandler/InvalidOrganizationChangeEventHandler.java
---
(0096436)
mtaal   
2017-05-07 21:38   
I think the solution should be to get rid of the commit in the MobileServerProvider.initializeDataCachedByOrganization. Especially because this cache initialization only reads data and does no updates/inserts.

Imho the commit should be done by callers, many callers can expect database actions to take place and should commit their transaction.

I saw that the StoreBusinessPartnerLoaderHook also calls the MobileServerProvider using the method which does a commit, this can give also unwanted effects. So I guess it seems better to research how to get rid of the commit in the cache-building logic and let the callers manage transactions.
(0096675)
hgbot   
2017-05-19 11:13   
Repository: erp/pmods/org.openbravo.replication.symmetricds
Changeset: adfb82ccf42286d731d4d7e68b766932f75b7d3e
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Fri May 19 11:13:27 2017 +0200
URL: http://code.openbravo.com/erp/pmods/org.openbravo.replication.symmetricds/rev/adfb82ccf42286d731d4d7e68b766932f75b7d3e [^]

Fixes issue 35897: Prevent doing a commit inside an event handler (InvalidOrganizationChangeEventHandler)
Let the transaction handling within the provider depend if there is already a transaction or not

---
M src/org/openbravo/replication/symmetricds/util/MobileServerProvider.java
---
(0096676)
hgbot   
2017-05-19 11:15   
Repository: erp/pmods/org.openbravo.retail.storeserver.synchronization
Changeset: 62bea10d1a32d26737ea26eb633048d95215205d
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Fri May 19 11:14:32 2017 +0200
URL: http://code.openbravo.com/erp/pmods/org.openbravo.retail.storeserver.synchronization/rev/62bea10d1a32d26737ea26eb633048d95215205d [^]

Related to issue 35897: Prevent doing a commit inside an event handler (InvalidOrganizationChangeEventHandler)
No need to pass the commit parameter as the MobileServerProvider knows if it is participating in a transaction

---
M src/org/openbravo/retail/storeserver/synchronization/eventhandler/InvalidOrganizationChangeEventHandler.java
M src/org/openbravo/retail/storeserver/synchronization/eventhandler/OrderEventHandler.java
---
(0096713)
AugustoMauch   
2017-05-22 15:36   
Code reviewed and verified
(0096825)
hgbot   
2017-05-26 08:06   
Repository: erp/pmods/org.openbravo.replication.symmetricds
Changeset: 3335ecfec5aeec4b55f1f1043c0b621e5876ed71
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Fri May 26 08:06:36 2017 +0200
URL: http://code.openbravo.com/erp/pmods/org.openbravo.replication.symmetricds/rev/3335ecfec5aeec4b55f1f1043c0b621e5876ed71 [^]

Related to issue 35897: Prevent doing a commit inside an event handler (InvalidOrganizationChangeEventHandler)
Add methods back for backward compatibility of modules

---
M src/org/openbravo/replication/symmetricds/util/MobileServerProvider.java
---
(0096826)
hgbot   
2017-05-26 08:07   
Repository: erp/pmods/org.openbravo.retail.giftcards.synchronization
Changeset: a40da9b10d534f9e54e24a90d6cbf391f6342fa3
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Fri May 26 08:07:17 2017 +0200
URL: http://code.openbravo.com/erp/pmods/org.openbravo.retail.giftcards.synchronization/rev/a40da9b10d534f9e54e24a90d6cbf391f6342fa3 [^]

Related to issue 35897: Prevent doing a commit inside an event handler (InvalidOrganizationChangeEventHandler)
Use the MobileServerProvider without commit parameter

---
M src/org/openbravo/retail/giftcards/synchronization/eventhandler/GiftCardInstanceEventHandler.java
---