Hi, I tried to build an automated test for two clients reserving a DVD. First, I start my server. Then, I ran this test.
Originally, the DVD (UPC= "327254654435") has 6 copies.
After the test, there should be 4 copies as the two clients rented two of them.
But the automated test keeps reading 6.
It seems like my automated test does not start the clients and perform the reserveDVD method in the setup.
Is this the right approach to build automated tests for RMI?
Yes. I ran the test in both debug mode and JUnit Application mode in Eclipse last night. I put a debugger breakpoint at the reserveDVD() in line 26 and 27.
Before I ran the test, I ran the standalone application and see that the movie had 6 copies. Then, I shut down the application. Then, I started the server.
Then, I ran the test in debug mode and JUnit mode.
I noticed that whenever a movie is rented in server mode, a log message will show up on the server command prompt.
But during the test, the log message does not show up.
And no matter how many times I ran the test, the copies is still 6.
I have a feeling that I am not doing right to let the JUnit test to connect to the server.
I'm really wondering if you debugged the application thoroughly. I even doubt if you took time to read the javadoc of each invoked method, because if you did you certainly would have spotted the problem.
And I took the time to bring your test case to the current standards (using annotations). You can run this code using JUnit 4. It has exactly the same behavior as your test case, but has fewer lines (and no need of conventions about naming your methods).
Hi, I finally know why the reserveDVD method does not decrease the number of DVD.
The reserveDVD only does the locking and put DVD in the reservations list. But there is another method, setCopy to update the DVD's copies.
This is a fine-grained design.
In the exam, I may choose to do course-grain design: reserveDVD handles reservation and setCopy, so that testing the code is easier. I may choose to use test-driven design to develop the code.
I think the exam does not specify which approach should be used: test-driven or design-driven approach.
I would expect that you make the effort to correctly use the provided API if you create some (test) classes that use a given API to get the desired results. Although the test completes and succeeds, you are not using the DBClient as intended.
If I run your code, the test needs more than 5 seconds to complete. If I make proper adjustments (to use DBClient as intended), the test finishes in less than 0.5 seconds!
This is the code from GuiController from Denny's DVD.
I think dvd should a shared object by multiple threads and the setCopy method should be synchronized.
If client1 and client 2 are running this rent method, a possible scenario will be:
1. reserveDVD is synchronized, it is ok.
2. threads can do concurrent read on the DVD and create its own instance of DVD, dvd object.
3. If dvd object has 6 copies, problem will occur. dvd.setCopy(6-1) will be executed by client 1 and 2. So, client 1 and 2 will get 5 copies and called modifyDVD. Even though modifyDVD is synchronized, both dvd copies = 5, which is expected to be 4.
Therefore, I think dvd should be a shared instance and should be synchronized.
Or, my other approach is to decrease the DVD copy by 1 inside the reserveDVD method.
reserveDVD will make sure that only 1 client at a time can get the lock on the dvd-object. So if 2 clients rent the same movie, 1 client will have the lock on the dvd, the other has to wait until 1st client has finished (= changed numer of copies and released the lock) before it can rent the same dvd
The problem is not from reserveDVD. This method is synchronized, which is good.
The getDVD method has concurrent read and mutual exclusive write, which is good.
But the the problem is from the setCopy() , which is not synchronized.
Here is my logic:
1. client 1 getDVD and client2 getDVD.
2. Both of them get the same dvd information and have their own local instance of DVD, dvd object.
3. If each DVD dvd object has the 6 copies, there is a chance that client 1 setCopy to 5 and client2 setCopy to 5 as well.
The reserveDVD and getDVD are synchronized, but the setCopy is not. And there is a chance that client 1 and client 2 concurrently call the setCopy method.
I am sure the Monkhouse book does a good job in term of using the concurrency API.
I really appreciate the explanation when it explains the lock and synchronize application in reserveDVD and getDVD.
I took a careful look at the rent method in GuiController.
When two threads (clients) execute the rent method concurrently, I think setCopy also be synchronized.
Suppose two clients rent the same DVD, here is possible scenario:
1. client 1 calls reserveDVD , which is ok.
2. client 2 calls reserveDVD, which is ok.
3. client 1 calls the getDVD, creates its own DVD dvd object and client 2 calls the getDVD, creates its own DVD dvd object. Client1 and 2 are getting DVD concurrently. So, they are reading the same DVD, which has 6 copies.
4. client 1 executes the setCopy(dvd.getCopy()-1), which is setCopy(6-1). Concurrently, client2 executes the setCopy(dvd.getCopy()-1), which is also setCopy(6-1);
(the dvd used by client1 and the other dvd used by client2 are two different local instances which has the same values).
That is why I think setCopy should also be synchronized. Two threads set setCopy to 5, but the result should be 4.
Maybe first have a look at the javadoc (and implementation) of the reserveDVD. That's why the scenario you proposed is impossible to happen.
Given your scenario, this is the correct version.
1. client 1 calls reserveDVD , which is ok. (client1 now has lock on the requested dvd)
2. client 2 calls reserveDVD. (the requested dvd is already locked, so client2 has to wait until lock on requested dvd is released)
3. client 1 calls the getDVD which has 6 copies. (client2 still waiting)
4. client 1 executes the setCopy(dvd.getCopy()-1) and modifies dvd (dvd is updated, client2 still waiting)
5. client 1 releases lock. (client2 is notified)
6. client 2 gets the lock on requested dvd
7. client 2 calls the getDVD which has 5 copies.
I looked at the code and the Javadoc.
The reserveDVD is perfectly fine and the getDVD is perfectly fine. The reserve method locks and unlocks. It is considered as an atomic operation.
The issue that I am thinking of comes from the setCopy.
The setCopy method is executed after the reserve method is unlocked.
Like you said in my other post. You said in my RmiTest.java, I should synchronized the setCopy in the test. That inspires me and raise this question.
I give it a last try: because of you MUST own a lock (which just 1 client at a time can get) before you can get the dvd (through getDVD-method) and modify it, it's IMPOSSIBLE for 2 clients to call the getDVD method for the same upc.
In my post above I gave you a corrected version of your scenario (because your scenario is impossible to happen if you call reserveDVD prior to getDVD and/or modifyDVD). That's the whole purpose of the reserveDVD (and releaseDVD) methods: logical record locking! That's also nicely explained in ScjdFaq.
This piece of code comes from the rent method in GuiController.
Firstly,reserveDVD locks, reserves and unlocks. This is perfect.
Secondly, getDVD locks for concurrent reading, two threads read, and unlocks.
Next, client 1 and client2 now have two local instances of DVD dvd. Since their instances of dvd have 6 copies of each, setCopy (5) will be executed by two threads concurrently.
Lastly, modifyDVD locks, modifies and unlock, which is good.
Therefore, reserveDVD, getDVD and modifyDVD are all synchronized and they are three atomic operations. However, dvd.setCopy(dvd.getCopy-1) is not synchronized for two threads.
If the rent method is synchronized, setCopy is executed in a synchronized manner. But the rent method is not synchronized.
The rent method contains three atomic operations, which are synchronized. However, the setCopy is not synchronized.
My focus is on the setCopy part in the above code, not the reserveDVD, getDVD or modifyDVD.
That is ok. It is actually a good chance to discuss details about the book.
By the way, no matter we use synchronized keywords or logical record locking, the code is considered as synchronized and thread safe.
I am focusing on every line of the rent method. I am sure reserveDVD, getDVD and modifyDVD methods are fine as atomic thread safe code.
I don't have any questions regarding to these three methods.
When I read KB's book, KB recommended us to synchronize (no matter using synchronized keyword or other concurrency API) a whole block to make the code thread safe. For example, Vector class has synchronized methods. But when we use vector objects, we need to consider synchronizing the whole piece of code.
According to KB, vector's add method is synchronized. But we need to make sure the code after vector.add should also be synchronized too to make it thread safe.
So, KB book suggests to synchronize the whole block of code.
The same issue happens if with the rent method here in psuedo code
I am interested to know if two clients rent the same DVD concurrently, what will be the logic flow?
The logical locking and unlocking occurs within three methods : reserveDVD, getDVD and modifyDVD.
But I don't see any logical locking within the setCopy method.
Even if you would make the setCopy-method synchronized you might end up with 2 threads setting the number of copies to 5 if you do not use logical record locking! It will NOT solve the issue you described in this post.