On *nix systems O_CREAT and O_EXCL are mutually exclusive and will 
prevent creation of a file if one already exists. Therefore lock needs 
to be created separately or you need to create another file on the same 
disk and then use atomic rename.

Excuse me, I don't intend to sound rude - but did you even read my report? I never even mentioned O_EXCL and it has NOTHING to do with the problem I reported.

To summarize the problem again (and perhaps make myself clearer):

file_put_contents has - according to the documentation - a third parameter called $flags. In the documentation, it is stated, that the LOCK_EX constant may be passed as a flag. The documentation for file_put_contents states:

> LOCK_EX: Acquire an exclusive lock on the file while proceeding to the writing.

So, one may assume that the file will be locked exclusively BEFORE writing to it.

Now, the problem is, that this is not the case! Have a look at ext/standard/file.c:
http://cvs.php.net/viewvc.cgi/php-src/ext/standard/file.c?revision=1.409.2.6.2.27&view=markup

There you see this line of code:

	stream = php_stream_open_wrapper_ex(filename, (flags & PHP_FILE_APPEND) ? "ab" : "wb", 
			((flags & PHP_FILE_USE_INCLUDE_PATH) ? USE_PATH : 0) | ENFORCE_SAFE_MODE | REPORT_ERRORS, NULL, context);

Followed by that line:

	if (flags & LOCK_EX && (!php_stream_supports_lock(stream) || php_stream_lock(stream, LOCK_EX))) {

So, what does this code do? (if FILE_APPEND was not specified)

1) It openes the file in 'wb' mode for writing.
2) It locks the file exclusively
3) THEN it actually starts writing the file contents

So, the problem actually is the following:

'wb' as a fopen(3) mode translates to O_WRONLY | O_CREAT | O_TRUNC as an open(2) mode. What does that do? It truncates the file UPON OPENING IT! And AFTER THAT it tries to acquire the lock. Since locks on POSIX-compatible systems are advisory (flock(2) anyway), any possible other lock on the file will NOT be honoured by file_put_contents and the $flag == LOCK_EX parameter is completely ineffective.

Please, have a look at my test case - it's really simple. One PHP script opens the file for reading, acquires a shared lock and goes to sleep. If the other PHP script is executed in the mean time, the file is opened and then the other script tries to acquire an exclusive lock on the same file - and has to wait until the first script releases it. That's fine. But what's not fine is that prior to acquiring the exclusive lock it has ALREADY modified the file! So after the first script returns from it's sleeping phase, it will see an empty file because it was truncated by the other script upon opening.

What the CURRENT PHP code translates to is basically:

int fd = open ('file.txt', O_WRONLY | O_CREAT | O_TRUNC, 0666);
flock (fd, LOCK_EX);

Which causes exactly the described problems.

And there IS a solution for this. The following C code CORRECTLY acquires an exclusive lock for writing to a file WITHOUT truncating it before it is safe to do so:

int fd = open ('file.txt', O_WRONLY | O_CREAT, 0666);
flock (fd, LOCK_EX);
ftruncate (fd, 0);
// now write something to fd

This is absolutely correct in POSIX and completely portable (assuming flock(2) is provided by the OS, but if fcntl(2) is used as a replacement, it does not change anything).

So now the ONLY question remains: How is it possible to integrate that fix with PHP code?

And that is a bit more tricky than the mere C code becaues PHP uses fopen(3)-style file modes for the stream wrapper API instead of numeric file modes as provided by open(2). And there is currently no fopen(3)-style file mode translating to O_WRONLY | O_CREAT.

So, I see two possible solutions:

 * Add another fopen(3) style file mode that does exactly that.
 * Remove the LOCK_EX flag from file_put_contents completely.

It DOES NOT make sense to keep the flag but leave this bug unfixed, because

 a) It is utterly and completely useless with advisory locking.
    Keeping it will only cause people who read the documentation
    to assume it's safe to use the flag - which is plainly WRONG.

 b) If mandatory locking is used (i.e. when using Windows), the
    OTHER LOCK that is already in place on the file will take care
    for the data consistency, i.e. a lock created by another
    program will delay the truncating of the file until AFTER the
    lock is released. The exclusive lock created on the file itself
    will have NO interaction with any other lock in place prior to
    the file_put_contents_call.

Oh, I forgot:

There's a solution for this problem that is quite a bit easier as the other one I suggested:

ALWAYS open the file in append mode and truncate it AFTER acquiring the lock. Normally this has the drawback that the append mode does NOT have the same semantics as the write mode (i.e. using fseek to reposition the write pointer won't work as the contents will always be appended to the end of the file) but that doesn't matter here since a) with ftruncate() the end of the file will be at byte zero and b) file_put_contents only writes out the complete data and does not need to reposition the write pointer of the file.

Well, at least with normal C and fopen(3) it's not that complicated, with PHP's streams API it gets complicated again because some stream wrappers don't support truncating of a file (e.g. the FTP wrapper). Those don't support locking either, so that wouldn't be a problem anyway except that it's as far as I could see not possible to determine whether truncating the file will be possible BEFORE opening it.

Anyway, here's a patch against the PHP 5.2 branch that fixes this bug:

===================================================================
RCS file: /repository/php-src/ext/standard/file.c,v
retrieving revision 1.409.2.6.2.28
diff -u -r1.409.2.6.2.28 file.c
--- ext/standard/file.c 4 Sep 2007 12:51:49 -0000       1.409.2.6.2.28
+++ ext/standard/file.c 6 Nov 2007 18:49:48 -0000
@@ -604,16 +604,44 @@

        context = php_stream_context_from_zval(zcontext, flags & PHP_FILE_NO_DEFAULT_CONTEXT);

-       stream = php_stream_open_wrapper_ex(filename, (flags & PHP_FILE_APPEND) ? "ab" : "wb",
+       // open in append mode and truncate later with LOCK_EX
+       // note that this will cause the stream to be closed and opened again
+       // if truncating is not supported - but there is no way with the
+       // current streams API to check before actually opening the file
+       stream = php_stream_open_wrapper_ex(filename, (flags & PHP_FILE_APPEND || flags & LOCK_EX) ? "ab" : "wb",
                        ((flags & PHP_FILE_USE_INCLUDE_PATH) ? USE_PATH : 0) | ENFORCE_SAFE_MODE | REPORT_ERRORS, NULL, context);
        if (stream == NULL) {
                RETURN_FALSE;
        }
+       // worst-case scenario: user wanted LOCK_EX but the stream can't be
+       // truncated. So, reopen in 'wb' mode and ignore locking
+       // (TODO: possibly generate a warning that LOCK_EX wasn't possible with
+       // this stream?)
+       if (!(flags & PHP_FILE_APPEND) && (flags & LOCK_EX) && !php_stream_truncate_supported(stream)) {
+               php_stream_close(stream);
+               stream = php_stream_open_wrapper_ex(filename, "wb",
+                               ((flags & PHP_FILE_USE_INCLUDE_PATH) ? USE_PATH : 0) | ENFORCE_SAFE_MODE | REPORT_ERRORS, NULL, context);
+               // remove LOCK_EX flag - locking will be useless now anyway
+               // (we just truncated the file)
+               flags = flags & ~LOCK_EX;
+       }

        if (flags & LOCK_EX && (!php_stream_supports_lock(stream) || php_stream_lock(stream, LOCK_EX))) {
                php_stream_close(stream);
                RETURN_FALSE;
        }
+
+       // if the file was to be locked and not to be opened in append mode,
+       // we now need to truncate it (we already checked for truncate support
+       // and reopened the stream and removed LOCK_EX if it wasn't available)
+       if (!(flags & PHP_FILE_APPEND) && (flags & LOCK_EX)) {
+               if (php_stream_truncate_set_size(stream, 0)) {
+                       // we couldn't truncate the stream even though
+                       // truncating was supported
+                       php_stream_close(stream);
+                       RETURN_FALSE;
+               }
+       }

        switch (Z_TYPE_P(data)) {
                case IS_RESOURCE:
===================================================================

It is a bit ugly because of the problem described above: It can only be determined whether it's possible to truncate the file after it was opened, so if it's not, it has to be closed and opened again. The function remains functional in any case, just causes a little additional overhead if a stream wrapper without file truncation support (e.g. FTP) AND LOCK_EX are used together.

So, you have four possibilities now:

1) Use the patch as-is.
2) Improve the patch by somehow detecting truncate support before opening the file (and therefore removing the need to reopen the file if such a wrapper is used in combination with LOCK_EX).
3) Add the already mentioned file mode that doesn't include O_TRUNC
4) Remove the LOCK_EX flag from file_put_contents alltogether.

Assigned to Ilia for dismissing without reading..

This bug has been fixed in CVS.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.