Openbravo Issue Tracking System - Retail Modules
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0035767Retail ModulesStoreServerpublic2017-04-12 12:132017-05-15 09:14
ReporterAugustoMauch 
Assigned Tomtaal 
PrioritynormalSeverityminorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOS5OS Version
Product Version 
Target VersionFixed in VersionRR17Q3 
Merge Request Status
Review Assigned ToAugustoMauch
OBNetwork customer
Support ticket
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary0035767: Review synchronized methods in mobile.core, some of them may not be needed
DescriptionThe fix of this issue [1] removed a synchronized clause from a method to get rid of the contention is was being causing.

There are other synchronized methods that should be reviewed, maybe some of them are not needed anymore. For instance the synchronized clause of this method [2] would not be needed if the attribute was initialized eagerly.

Note that we must support a clustered environment, so a synchronized block cannot guarantee that a method is not executed concurrently in different application instances.


[1] https://issues.openbravo.com/view.php?id=35754 [^]
[2] https://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/file/c0b32b2cd2e2/src/org/openbravo/mobile/core/servercontroller/MobileServerRequestExecutor.java#l214 [^]
Steps To Reproduce-
Proposed Solution
Additional Information
TagsNo tags attached.
Relationships
related to defect 0035754 closed AugustoMauch MobileServerController.getThisServerDefinition creates contention 
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2017-04-12 12:13AugustoMauchNew Issue
2017-04-12 12:13AugustoMauchAssigned To => StoreServer
2017-04-12 12:13AugustoMauchTriggers an Emergency Pack => No
2017-04-12 12:13AugustoMauchRelationship addedrelated to 0035754
2017-05-01 17:20mtaalAssigned ToStoreServer => mtaal
2017-05-06 09:13mtaalNote Added: 0096433
2017-05-06 09:17mtaalReview Assigned To => AugustoMauch
2017-05-06 09:18mtaalStatusnew => acknowledged
2017-05-06 09:18mtaalStatusacknowledged => scheduled
2017-05-06 09:18mtaalNote Added: 0096435
2017-05-06 09:18mtaalStatusscheduled => resolved
2017-05-06 09:18mtaalFixed in Version => RR17Q3
2017-05-06 09:18mtaalResolutionopen => fixed
2017-05-08 09:09hgbotCheckin
2017-05-08 09:09hgbotNote Added: 0096446
2017-05-08 09:11hgbotCheckin
2017-05-08 09:11hgbotNote Added: 0096447
2017-05-15 09:12AugustoMauchDescription Updatedbug_revision_view_page.php?rev_id=15163#r15163
2017-05-15 09:14AugustoMauchNote Added: 0096583
2017-05-15 09:14AugustoMauchStatusresolved => closed

Notes
(0096433)
mtaal   
2017-05-06 09:13   
See commit, also in clustered environments synchronized methods can make sense to initialize in-memory information in a correct way or preventing threads or other things to be started concurrently.
(0096435)
mtaal   
2017-05-06 09:18   
Fixed by this commit:

https://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/ff07f507ed74 [^]
(0096446)
hgbot   
2017-05-08 09:09   
Repository: erp/pmods/org.openbravo.retail.storeserver.synchronization
Changeset: 347ee3bf76175ffb4ba52a9042ab0f13493002fe
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Mon May 08 09:09:36 2017 +0200
URL: http://code.openbravo.com/erp/pmods/org.openbravo.retail.storeserver.synchronization/rev/347ee3bf76175ffb4ba52a9042ab0f13493002fe [^]

Related to issue 35767: Review synchronized methods in mobile.core, some of them may not be needed
Removed synchronized statement as not needed here

---
M src/org/openbravo/retail/storeserver/synchronization/eventhandler/OrderEventHandler.java
---
(0096447)
hgbot   
2017-05-08 09:11   
Repository: erp/pmods/org.openbravo.retail.storeserver.synchronization
Changeset: 00bf2946bcc1f6ceee38ceb1d5997d2dc7512aff
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Mon May 08 09:10:42 2017 +0200
URL: http://code.openbravo.com/erp/pmods/org.openbravo.retail.storeserver.synchronization/rev/00bf2946bcc1f6ceee38ceb1d5997d2dc7512aff [^]

Related to issue 35767: Review synchronized methods in mobile.core, some of them may not be needed
Removed synchronized and use mobile server definition id instead of reading object itself

---
M src/org/openbravo/retail/storeserver/synchronization/StoreServerVersionHandler.java
---
(0096583)
AugustoMauch   
2017-05-15 09:14   
Code reviewed and verified