[Lock] Strange pattern

[Olivier Gregoire]

Hi,

My current tests on the locking system always shows the same pattern, and I don't know if it's right or wrong: all updates, and deletes on the record 1 are done at the end of the test, except one or two. I've tested it and seen it on two different systems (one Windows, one Linux).

The problem is that I really don't know how to interpret this. Does anyone have the same pattern?

Here's my test method adapted from the one present in the FAQ:

	@Test(timeout = 1000)
	public void shouldNotDeadlock() {
		List<Thread> threads = new ArrayList<Thread>(50);
		
		for (int i = 0; i < 10; i++) {
			threads.add(new UpdatingRandomRecordThread());
			threads.add(new UpdatingRecord1Thread());
			threads.add(new CreatingRecordThread());
			threads.add(new DeletingRecord1Thread());
			threads.add(new FindingRecordsThread());
		}
		
		for (Thread thread: threads) {
			thread.start();
		}
	}

And here's a sample of the test result.

18: record created
15: record found
13: record created
17: record 1 updated
25: record found
26: record 22 updated
20: record found
23: record created
28: record created
21: record 33 updated
30: record found
31: record 21 updated
33: record created
35: record found
43: record created
45: record found
36: record 34 updated
40: record found
38: record created
46: record 5 updated
53: record created
48: record created
16: record 25 updated
50: record found
55: record found
58: record created
56: record 20 updated
24: record 1 deleted
60: record found
22: record 1 updated
59: record 1 deleted
19: record 1 deleted
34: record 1 deleted
47: record 1 updated
12: record 1 updated
11: record 43 updated
57: record 1 updated
29: record 1 deleted
54: record 1 deleted
52: record 1 updated
44: record 1 deleted
51: record 25 updated
49: record 1 deleted
42: record 1 updated
27: record 1 updated
32: record 1 updated
14: record 1 deleted
39: record 1 deleted
41: record 41 updated
37: record 1 updated

[Roel De Nijs]

Hi Olivier,

I would expect that a deleted record will not be updated, I would expect a RNFE being thrown instead.

Kind regards,
Roel

[Olivier Gregoire]

That's not an issue right now, since all methods are empty, except about the lock.

Regarding the strange pattern, is this normal? Or should I already have to correct the lock system? I prefer to work one thing at a time, and started with the lock.

[Roel De Nijs]

Hi Olivier,

Sorry, didn't know about the empty methods

I think it's a bit weird, but it could just be fine. Maybe you have to shuffle your list with threads a bit and expirement a bit with it

Kind regards,
Roel

[Olivier Gregoire]

Indeed, you couldn't know about the empty methods. I'll try to be clearer next times.

I'll try and use the shuffle but I don't really think it's gonna change much, since these are threads, and the execution is non-predictable, even if we add randomness.

[Roel De Nijs]

Olivier Gregoire wrote:since these are threads, and the execution is non-predictable, even if we add randomness.

That's true of course, but it is strange that all these calls are at the end, but it could be completely valid behavior. Hard to say of course without any explanation (pseudo-code) how you make your Data class thread-safe. Are you already using lock/unlock methods?

Kind regards,
Roel

[Olivier Gregoire]

The implementation that I chose for this is the basic Lock implementation. It is contained directly in my Data class. But now with the issue I'm facing, I'm thinking about the default implementation of the ReadWriteLock. I'll implement the other lock too and check if everything is ok.

Roughly, I perform the lock only in the test classes used in my JUnit.

It's simply using the DB api:

long cookie = this.db.lock(recNo);
this.db.update(recNo, data, cookie); // or delete or...
this.db.unlock(recNo, cookie);

I don't really know how I can go deeper in the explanation regarding the lock system itself without copy/pasting the code. All I could say without revealing much is that my method pattern is the following:

long lock (int recNo) {
  // perform the synchronization lock
  try {
    // perform checks
    // do lock the record.
    return cookie;
  } finally {
    // perform the synchronization unlock.
  }
}
void unlock (int recNo, long cookie) {
  // perform the synchronization lock
  try {
    // perform checks
    // do unlock the record
    return;
  } finally {
    // perform the synchronization unlock.
  }
}

If I already wrote too much, please tell me so I remove the code.

[Roel De Nijs]

Hi Olivier,

Your pseudo code snippets seem ok to me. Although I'm not a big fan of the return; statement in the unlock-method. I didn't use the fancy classes from the concurrent package, I just used synchronized (on each method), wait and notify. But I believe most people use a ReadWriteLock.

Kind regards,
Roel