Win a copy of The Little Book of Impediments (e-book only) this week in the Agile and Other Processes forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Monkhouse book: Threading issue in persistDVD method?

 
Robert McDonald
Greenhorn
Posts: 17
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi all,

Whilst reading the code from Andrew's book, i've noticed what i think could be a problem with the persistDVD method and was wondering if anyone could explain it.

In the persistDVD method the code attempts to get the offset by using the database file length() method. This is done in a writeLock so it is threadsafe during this process. However, just after this the writelock is unlocked, then there is some processing of the data and then the code is synchronised whilst the seek to the offset is done and the file is written to.

But what about this scenario:
1) thread A calls persist with a new record, gets the (end of file) offset which is say 1000 moves out of the lock into the processing code.
2) thread B comes into the method to create a new record, gets the (end of file) offset (still 1000) moves out of the lock into the processing code.
3) Meanwhile thread A moves into the synchronised code to update the datafile at position 1000 and then exits the method.
4) thread B now moves into the synchronised code and updates the file BUT also at position 1000 - effectively overwriting the entry which A has just written.

Does this whole section (from determination of the offset to the db file write) not need to be in a synchronised context to avoid the above situation?

Maybe i'm missing something so if i am, I'd appreciate some pointers as to what is going on in the code and how it is threadsafe.

Thanks
 
Shlomo Hillel
Greenhorn
Posts: 14
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Oh My God! This is exactly what I was about to write!!!
Thanks for saving me the time to articulate this problem!

I think you are right. This is a problem as there are two views of the records and they need to be consistent.

Monkhouse, can you defend your code?
 
Andrew Monkhouse
author and jackaroo
Marshal Commander
Pie
Posts: 12014
220
C++ Firefox Browser IntelliJ IDE Java Mac Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Robert & Shlomo,

You are correct - there is a logical loophole here which could allow two clients to create a record at the same location in the file.
Does this whole section (from determination of the offset to the db file write) not need to be in a synchronised context to avoid the above situation?
Part of the purpose of that section of the code was to demonstrate how to avoid blocking clients. There are several possible solutions here, for example:
  • You could synchronize the entire persistDvd method.


  • The disadvantage of this is that only one client can ever be in the persistDvd method, which dramatically lowers concurrency.

  • You could synchronize within the addDvd() method so that only one client can ever be creating a record at a time.


  • This is slightly better, but still lowers concurrency.

  • Since the recordNumbers collection keeps track of offsets into the file, we could use that fact to track where records are being added.
  • To explain this further, the offending code is:If we were to change this to:We will ensure that two clients cannot write to the same location, without unduly reducing concurrency.

    Regards, Andrew
     
    • Post Reply
    • Bookmark Topic Watch Topic
    • New Topic