TObjectListLocked changed, creating dead locks

sakura · 2020-03-09 09:40:30

Hi,

so far, it was safe to do following:

  LockedList.Safe.Lock;
  try
    for ItemIdx := 0 to LockedList.Count - 1 do
      ItemClear(TMypSession(LockedList[ItemIdx]));

    LockedList.Clear;
  finally
    LockedList.Safe.UnLock;
  end;

with the recent change, having seen that in many places, it will dead lock at the clear method.

Further, having methods like Add/Remove/... automatically thread safe, and others like Delete not being thread safe is, imho, not a good idea, as one ALWAYS has to look at the documentation, to be sure what's happining. And as the documemantion does not work within the IDE, that means jumping around in the sources.

THe old SafeAdd/Safe... methods where more self-identifying and certainly easy to use.

Please, roll back that change. :-)

ab · 2020-03-09 11:13:57

LockedList.Clear would never make a race condition.
It calls the same LockedList.Safe lock, which is reentrant.
If you have a race condition, it doesn't comes from this change.

sakura · 2020-03-09 11:29:22

So Safe.Lock allows me to "lock" multiple times from within the same thread without deadlocking? It only blocks other threads at the same time? Didn't know/test that before. Would be cool.

Still leaves, imo, the big problem, that some methods do locking automatically (like Add/Remove/Clear), while others like Delete do not, which is not obvious, when using the list.

ab · 2020-03-09 11:38:34

@sakura
Yes, this is how TRTLCriticalSection works: they are re-entrant by definition.
Delete() from an index can not be thread-safe, because it uses an index which may change if something is added in a background thread. An explicit lock is needed.
Remove() make a search + delete so it can be made thread-safe.

sakura · 2020-03-09 11:43:45

Okay, thx.

Still feels a bit funny, but I understand. Have a real problem now, will check a bit further and likely will open another thread in a minute.

macfly · 2020-03-09 12:48:53

I particularly like the pattern that returns the locked instance. Then, you can always block and release the necessary access to the list.
It is very simple to implement as you do not need to override the methods in the original list.

with MyList.Locked do //Locked make the lock and returns the FList itself
try
 Add(..);
 Delete(..)
 Clear;
finally
  MyList.unlock;
end;

But the implementation of mORMot is also very good.

Last edited by macfly (2020-03-09 12:50:55)

ab · 2020-03-09 13:02:06

"with" is evil.

George · 2020-07-07 15:45:50

Is there an option to allow multiple read single write logic?
Like TMultiReadExclusiveWriteSynchronizer do.
Useful when object must be thread safe with big amount of read operations and rare write operations.

Last edited by George (2020-07-07 15:51:14)

ab · 2020-07-07 20:52:27

Not yet. Use TMultiREWS if you need to.

trx · 2020-07-08 08:10:38

George wrote:

Is there an option to allow multiple read single write logic?
Like TMultiReadExclusiveWriteSynchronizer do.
Useful when object must be thread safe with big amount of read operations and rare write operations.

I usually use TPasMPMultipleReaderSingleWriterLock from PasMP which works with both Delphi and FPC (Windows and Linux).

Last edited by trx (2020-07-08 08:11:32)

mORMot Open Source

#1 2020-03-09 09:40:30

TObjectListLocked changed, creating dead locks

#2 2020-03-09 11:13:57

Re: TObjectListLocked changed, creating dead locks

#3 2020-03-09 11:29:22

Re: TObjectListLocked changed, creating dead locks

#4 2020-03-09 11:38:34

Re: TObjectListLocked changed, creating dead locks

#5 2020-03-09 11:43:45

Re: TObjectListLocked changed, creating dead locks

#6 2020-03-09 12:48:53

Re: TObjectListLocked changed, creating dead locks

#7 2020-03-09 13:02:06

Re: TObjectListLocked changed, creating dead locks

#8 2020-07-07 15:45:50

Re: TObjectListLocked changed, creating dead locks

#9 2020-07-07 20:52:27

Re: TObjectListLocked changed, creating dead locks

#10 2020-07-08 08:10:38

Re: TObjectListLocked changed, creating dead locks

Board footer