I am having a servlet filter and within that I need to make some code as thread safe.
I am giving the abstract code:
doFilter() {
{
......
if (condition1) {
TestClass testObj = StaticTestClass.getTestObj();
testObj = testObj.setTestStr(testObj.getTestStr() + "Success");
StaticTestClass.setTestObj(testObj);
}
.....
}
I want to make it thread safe. condition1 will be true very rarely and hence there will be negligible performance hit because of synchronization. So I can do any of the following:
doFilter() {
{
......
if (condition1) {
TestClass testObj = StaticTestClass.getTestObj();
synchronized(this) {
testObj = testObj.setTestStr(testObj.getTestStr() + "Success");
StaticTestClass.setTestObj(testObj);
}}
......
}
or
doFilter() {
{
......
if (condition1) {
TestClass testObj = StaticTestClass.getTestObj();
synchronized(testObj) {
testObj = testObj.setTestStr(testObj.getTestStr() + "Success");
StaticTestClass.setTestObj(testObj);
}}
......
}
As per my understanding, conceptually the second one is more accurate as it is taking the lock of testObj. But the first one will also be correct as there will be only one instance of a servlet filter in the container.
Please let me know if anyone is having different opinion.
Your understanding is correct: The second option is the more correct of the two, because the goal of the synchronized statements is to access testObj, not this.
If at some point in the future you need to implement new features in your app and add other accesses to testObj, you will have to make that synchronization on testObj, and not on this. So you might as well make it so from the start.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With