TYPO3 Core

v2: split into three files (one for each class).

related also to: http://bugs.typo3.org/view.php?id=8010 ,

especially as there has been a whish to split each in a separate class.

Maybe it would be easier to get this thing splitted in two separate patches:

1) splitting up without changing the functionality

2) fixing the race condition part

I think mixing both causes some confusion in the reviewing process (see comment from Dmitry).

And how does it relate to #18557? Will you be able to integrate Olly's addition in your splitup?

But maybe I understood something wrong, as I haven't really read through the code.

Just to summarize current status and my changes:

currently we have t3lib_lock with all the switches in it.

I introduced a new abstract lock class ( copying the current t3lib_lock and only removing the acquire and release methods) gets extended to get a functionally equivalent t3lib_lock ( with the old acquire and release code) again.
Additionally I introduced a second lock class t3lib_lock_static, also extending the abstract class with release and acquire method.

The static lock does not remove it's ressources, so it can be used as a global lock.
To get rid of the race condition t3lib_lock got an instance ( stored in $globals ) of the static class to protect access to it's lock ressource.

It sounds like you have:

- part 1 is a refactoring of t3lib_lock into an abstract and a (backwards compatible) concrete class
- part 2 is a bugfix in t3lib_lock
- part 3 is a new feature t3lib_lock_static

So it would be very great to have this split up in three patches that does that in that order so that the history of changes reflect that.

Maybe before you do that work, we should clarify with Olly if we could have the t3lib_lock_Database (#18557) using your refactoring too.

About the naming and file location this is more or less how our CGL tells us to do it:

• t3lib_lock = t3lib/class.t3lib_lock.php
• t3lib_lock_AbstractLock = t3lib/lock/class.t3lib_lock_abstractlock.php
• t3lib_lock_Static = t3lib/lock/class.t3lib_lock_static.php
• t3lib_lock_Database = t3lib/lock/class.t3lib_lock_database.php

I can integrate patch from Olly in my current rewrite. If there is a decision to generate new sub-classes, it will take more work and reviews.

Hi Olly, you might want to take a look at this issue. What do you think of this? Could this idea of the abstract lock be re-used or made more generic? Maybe we should create a LockInterface above that all?

- part 1 is a refactoring of t3lib_lock into an abstract and a (backwards compatible) concrete class
- part 2 is a new feature t3lib_lock_static (needed for the bug fix)
- part 3 is a bugfix in t3lib_lock

For the classes:
The split-up suggested by Dimitry would give
t3lib_lock : A proxy for the sub-classes
t3lib_lock_Simple
t3lib_lock_Flock,
t3lib_lock_Seamphore
t3lib_lock_Database

and static variants of all sub-classes.

3a fixes a minor typo

Currently postponing it to4.6. I found some problems with non-triggering autoloader.

Some more remarks for my patch:

• The available locking methods are hardcoded, they should probably be declared in TYPO3CONF so that extensions can add locking methods.
• The return value of acquire seems still senseless to me. The information wether we had to wait for lock or not is not really useful, a success/failure in indication would be more useful (like a typical posix mutex).

I will probably not have much time to work on teh code the next days.

Tanks for your ground work so far.

Björn Pedersen wrote:

After looking at it, there still some things not optimal:

Right. I've seen it. I'll take care of it. I already did some cleanup of your patch already but I'm not yet happy with the result. Once I'm ready, I'll push it to Gerrit.

@Helmut:
Any news on this one?