2

I had asked a question earlier( How to keep this counter from reseting at 100,000? ), and now have a follow-up question.

I have another version of the counter in question that can be told to reset at a certain number, and I would like to make sure that this second version does not have the same problem as the first.

What I have coded now is:

$reset = '10';

$filename4 = "$some_variable/$filename3.txt";  

// Open our file in append-or-create mode.
$fh = fopen($filename4, "a+");

if (!$fh)
    die("unable to create file");

if ($reset == 'default'){        
    // Before doing anything else, get an exclusive lock on the file.
    // This will prevent anybody else from reading or writing to it.
    flock($fh, LOCK_EX);

    // Place the pointer at the start of the file.
    fseek($fh, 0);

    // Read one line from the file, then increment the number.
    // There should only ever be one line.
    $current = 1 + intval(trim(fgets($fh)));

    // Now we can reset the pointer again, and truncate the file to zero length.
    fseek($fh, 0);

    ftruncate($fh, 0);

    // Now we can write out our line.
    fwrite($fh, $current . "\n");

    // And we're done.  Closing the file will also release the lock.
    fclose($fh);                
}
else {        
    $current = trim(file_get_contents($filename4)) + 1;

    if($current >= $reset) {            
        $new = '0';            
        fwrite(fopen($filename4, 'w'), $new);                        
    }
    else {            
        fwrite(fopen($filec, 'w'), $current);                        
    }               
}

echo $current;

I did not want to assume I know what changes to make to this code, so I post another question. EDIT- What changes should I make here to avoid not getting an exclusive lock on the file if $reset is not equal to default? What is the correct way to code this? Would this work?:

$filename4 = "$some_variable/$filename3.txt";  

// Open our file in append-or-create mode.
$fh = fopen($filename4, "a+");

if (!$fh)
die("unable to create file");


// Before doing anything else, get an exclusive lock on the file.
// This will prevent anybody else from reading or writing to it.
flock($fh, LOCK_EX);

// Place the pointer at the start of the file.
fseek($fh, 0);



if ($reset == 'default'){

// Read one line from the file, then increment the number.
// There should only ever be one line.
$current = 1 + intval(trim(fgets($fh)));

} else {
// Read one line from the file, then increment the number.
// There should only ever be one line.
$current = 1 + intval(trim(fgets($fh)));

if($current >= $reset) {            
    $current = '0';            
}
else {            
// Read one line from the file, then increment the number.
// There should only ever be one line.
$current = 1 + intval(trim(fgets($fh)));

} 
}
// Now we can reset the pointer again, and truncate the file to zero length.
fseek($fh, 0);

ftruncate($fh, 0);

// Now we can write out our line.
fwrite($fh, $current . "\n");

// And we're done.  Closing the file will also release the lock.
fclose($fh);                


echo $current;

EDIT - This seems to be working for me:

$reset = "default";
$filename4 = "counter.txt";  

// Open our file in append-or-create mode.
$fh = fopen($filename4, "a+");

if (!$fh)
die("unable to create file");


// Before doing anything else, get an exclusive lock on the file.
// This will prevent anybody else from reading or writing to it.
flock($fh, LOCK_EX);

// Place the pointer at the start of the file.
fseek($fh, 0);

// Read one line from the file, then increment the number.
// There should only ever be one line.
$current = 1 + intval(trim(fgets($fh)));

if ($reset == 'default'){
$new = $current;
} else {
if($current >= ($reset + '1')) {            
$new = '1';            
}
else {            
$new = $current;
} 
}
// Now we can reset the pointer again, and truncate the file to zero length.
fseek($fh, 0);

ftruncate($fh, 0);

// Now we can write out our line.
fwrite($fh, $new . "\n");

// And we're done.  Closing the file will also release the lock.
fclose($fh);                


echo $new;

Does this look right?

Community
  • 1
  • 1
mobilestimulus
  • 180
  • 1
  • 14
  • Sorry about the indentation. Neatness has never been my talent... – mobilestimulus Mar 09 '11 at 20:04
  • 1
    where does the value of $reset come from? This code basically says "if $reset = default, increment the value in the file by 1". "otherwise, if the current value in the file is greater than $reset, set current value to 0. – David Mar 09 '11 at 20:08
  • @David Thats right. $reset would have been set earlier in the page source. – mobilestimulus Mar 09 '11 at 20:10
  • @Mario As posted above, "What changes should I make here?" – mobilestimulus Mar 09 '11 at 20:11
  • 1
    So, um, you want arbitrary code suggestions? Or should the suggested changes have a specific outcome? – mario Mar 09 '11 at 20:13
  • @mario It would be great to try any suggestion. The hardest part for me seems to be communicating the question correctly. Of course if $reset is equal to default on that particular page, I just want to keep incrementing the counter. I got good advice in my first question, and posted the link to it above. I hope that helps make things more clear. – mobilestimulus Mar 09 '11 at 20:29
  • @mario I have also edited my question above to help clarify things, I hope... – mobilestimulus Mar 09 '11 at 21:04
  • Your edit is working for you because you (1) set $result = "default", and then (2) immediatly say "if $result == default, increment my counter. why even bother checking the value of $result? Can you confirm that your goal is simply to have a counter that increments and never gets reset? If so, this could have been a far easier q&a session. – David Mar 09 '11 at 22:51

3 Answers3

2
    if($current >= $reset) {
        // here is where you are setting the counter back to zero. comment out
        // these lines.
        //$new = '0';            
        //fwrite(fopen($filename4, 'w'), $new);                        
    }

If you simply want a counter that doesn't get reset, try:

$filename4 = "counter.txt";  

// Open our file in append-or-create mode.
$fh = fopen($filename4, "a+");

if (!$fh)
die("unable to create file");


// Before doing anything else, get an exclusive lock on the file.
// This will prevent anybody else from reading or writing to it.
flock($fh, LOCK_EX);

// Place the pointer at the start of the file.
fseek($fh, 0);

// Read one line from the file to get current count.
// There should only ever be one line.
$current = intval(trim(fgets($fh)));

// Increment
$new = $current++;

// Now we can reset the pointer again, and truncate the file to zero length.
fseek($fh, 0);

ftruncate($fh, 0);

// Now we can write out our line.
fwrite($fh, $new . "\n");

// And we're done.  Closing the file will also release the lock.
fclose($fh);

echo $new;                
David
  • 2,550
  • 7
  • 27
  • 29
  • 1
    near the end, you are comparing $current to $reset. If $current is larger than 10, in this case, you will set whatever number is in your file back to 0. By commenting out these lines (or better yet just remove teh whole if block), you will not reset the value to 0. – David Mar 09 '11 at 20:18
  • @David I am not sure I understand how this would work out. Can you be more clear in you explanation? Can I send private messages to other members here? – mobilestimulus Mar 09 '11 at 20:35
  • 1
    We meet again. @David. I think he wants to reset it to 0 in this case. The code is mostly right if that is the intention. – frostymarvelous Mar 09 '11 at 20:48
  • @frostymarvelous Am glad to see you here! Yes! You know what this code is for. That is my intention. – mobilestimulus Mar 09 '11 at 20:51
  • 1
    I didnt read the main processing of the text file, but the part where you check for the condition and reset is correct. – frostymarvelous Mar 09 '11 at 20:56
  • @frostymarvelous Thanks. I have edited my question above. I almost understand the answer..... – mobilestimulus Mar 09 '11 at 21:02
1

The best way I can see to do this would be to open the file for reading with a lock other than exclusive. you can then perform your required checks and if the the count exceeds the $reset value, you can close the the file, open it again but this time with the exclusive lock for writing. Another way would simply not to use an exclusive lock. You could look into very good flatfile classes out there which have tested locking mechanisms.

frostymarvelous
  • 2,786
  • 32
  • 43
1

file_put_contents is already atomic. There is no need for ten lines of file locking code.

<?php

    $fn = "$filename3.txt"; 

    $reset = 0;        // 0 is equivalent to "default"
    //$reset = 10000000;  

    $count = file_get_contents($fn);        
    $count = ($reset && ($count >= $reset)) ? (0) : ($count + 1);
    file_put_contents($fn, $count, LOCK_EX);

    echo $count;

No idea if this is any help, since your question is still opaque. I will not answer comments.

mario
  • 144,265
  • 20
  • 237
  • 291
  • 1
    TIL about the third argument to `file_put_contents`. That said, the OP was suffering from massive concurrency issues which mandated the lock. Take a look at the original question he linked. While your code shouldn't ever suffer from the race condition that caused *complete* data loss, your code is still vulnerable to losing data during multiple concurrent requests. – Charles Mar 09 '11 at 21:58
  • 1
    @Charles: Yes, it's a pretty inexact approach. If he wanted something extremely atomic, then `file_put_contents("$fn", "*", FILE_APPEND)` and `filesize($fn)` would do (with wasting accordant disk space of course). From the original question I assumed it was more crucial logic problems. – mario Mar 09 '11 at 22:18
  • 1
    @mobilestimulus, this answer is pretty suitable *for the reset code only*. – Charles Mar 09 '11 at 22:28
  • @Charles - I have edited my question post to reflect my current code and status. Does this look acceptable in it's functioning? – mobilestimulus Mar 09 '11 at 22:32