• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Is my cookie generator too simple?

 
David Chan
Ranch Hand
Posts: 30
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Since my lock-unlock mechanism is performed in server side, my thought is that the cookie value is no need to set very long to prevent the cookie duplication (one sent from clientA and another sent from clientB). But if I generate the cookie just by increment by one, is there any problem if I used with my lock method? Is this cookie generator OK?
public class CookieGenerator {
private static long cookie = 1;
public static long getNextCookie() {
return cookie++;
}
}
public long lockRecord(long recNo)
throws RecordNotFoundException {
synchronized(lockedRecords) {
try {
if(!isValidRecord(recNo))
throw new RecordNotFoundException();
long cookie = CookieGenerator.getNextCookie();
lockedRecords.put(new Long(recNo), new Long(cookie));
return cookie;
} catch(IOException ioe) {
throw new RuntimeException(ioe);
} catch(InterruptedException ire) {
throw new RuntimeException(ire);
}
}
}

David
 
Terry Martinson
Ranch Hand
Posts: 293
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
David -
Seems like you would be ok since you are only using the getNextCookie method in one place in that class, and its within a synchronized block.
See this thread for more information on a couple other options like the random number generator, which is what I use.
TJ
 
John Smith
Ranch Hand
Posts: 2937
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

public class CookieGenerator {
private static long cookie = 1;
public static long getNextCookie() {
return cookie++;
}
}

The "++" operation on a long is not atomic. It is possible for thread A to preempt thread B that is in process of incrementing your cookie. As a result, the two threads may get the same value of cookie. The getNextCookie() method needs to be synchronized.
 
Terry Martinson
Ranch Hand
Posts: 293
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
David and Eugene -
I'm trying to really understand this stuff. I agree with Eugene that it might be wise to synchronize the getNextCookie method. However, I'm still not convinced that it is really needed. Although the ++ is not atomic, wouldn't David still be ok since he is only calling this from within a synchronized block?
Example:
Thread A gets the first lock on his lockedRecords object
Thread A calls the getNextCookie method
Thread B preempts Thread A
In this case, Thread B would not be able to get a lock on the lockedRecords object, and would never get to the point of calling the getNextCookie method. So, it seems like 2 threads would not get the same value of the cookie.
Am I missing something here?
I really want to understand if the sync is strictly needed or not in this case.
Help???
(Just to reiterate though, either way I do think it would be wise to synch the method so as to "not confuse a jr. programmer")
TJ
 
David Chan
Ranch Hand
Posts: 30
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks for reply from both of you.
getNextCookie() without synchronized: if just look at CookieGenerator, grader may think the lock-unlock is totally failed due to "++" operation -> automatic failure
getNextCookie() is synchronized: grader may think the lock method is over-synchronized -> bad consideration. Can all graders concentrated enough to notice getNextCookie() in a synchronized block?
I am confused when it is considered as over-synchronized...

David
 
John Smith
Ranch Hand
Posts: 2937
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
In this case, Thread B would not be able to get a lock on the lockedRecords object, and would never get to the point of calling the getNextCookie method. So, it seems like 2 threads would not get the same value of the cookie.
Am I missing something here?
I really want to understand if the sync is strictly needed or not in this case.

In the usage of getNextCookie() method as posted above (within the synchronized section around the lockedRecords object) there is no problem. Notice, however, that getNextCookie() returns a local variable and therefore doesn't need to be within the synchronized section, -- it could be moved out of it, since each thread keeps its own stack where the local variables are kept. But when you move it out, your implementation of the CookieGenerator class is not thread safe anymore.
So, this is what I am pointing out to you, -- depending on how you use your CookieGenerator class, it may or may not be thread safe. A more robust design would make it thread safe no matter how it is used, and all you need for that is to synchronize the getNextCookie() method, and move its invocation out of the code that synchronizes on the other object.
 
Peter Yunguang Qiu
Ranch Hand
Posts: 99
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I wonder if it is necessary to use a class CookieGenerator, How about the following code:
public class Data implements DBAccess {
private static long cookie = 1;
............
............
public long lockRecord(long recNo)
throws RecordNotFoundException {
synchronized(lockedRecords) {
try {
if(!isValidRecord(recNo))
throw new RecordNotFoundException();
//---------------remove next line-----------
//long cookie = CookieGenerator.getNextCookie();
lockedRecords.put(new Long(recNo), new Long(cookie));
//return cookie; -------------replace it with next line----------
return ++cookie;
} catch(IOException ioe) {
throw new RuntimeException(ioe);
} catch(InterruptedException ire) {
throw new RuntimeException(ire);
}
}
}
}//end of class Data

any comments?
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic