Problem with deleting directories on NFS in FreeBSD

Problem with deleting directories on NFS in FreeBSD

by Marcin K. -
Number of replies: 3

Hi,

I've tried to open issue on https://tracker.moodle.org/ but the system does not want to accept it sad So I've decided to open topic on this forum. Maybe someone will find the problem important.

We have moodle installation on FreeBSD and recently we moved moodledata to NFS. After this we observed a problem with deleting directories from moodle.

I've started a topic on FreeBSD Forums: Problem wiith removing folders on NFS

The conclusion from the discusion is that this is definitely a problem of NFS client in FreeBSD but it can be solved at the Moodle's side. I think safely and system independently.

The essence of the problem is that directory handle changes while the entries are enumarated and deleted one by one. So one of the proposed solutions from the mentioned FreeBSD Forums topic was: list the directory content to the memory first and then delete all entries.

So I changed in moodlelib.php the contents of remove_dir function.

The originally there was:

    if (!$handle = opendir($dir)) {
        return false;
    }
    $result = true;
    while (false!==($item = readdir($handle))) {
        if ($item != '.' && $item != '..') {
            if (is_dir($dir.'/'.$item)) {
                $result = remove_dir($dir.'/'.$item) && $result;
            } else {
                $result = unlink($dir.'/'.$item) && $result;
            }
        }
    }

    closedir($handle);

I changed this to:

    if (!$handle = opendir($dir)) {
        return false;
    }
    $result = true;
    // First list all files in the directory ...
    $dirlist = [];
    while (false!==($item = readdir($handle))) {
        $dirlist[] = $item;
    }
    closedir($handle);
    // ... then delete all elements
    foreach ($dirlist as &$item) {
        if ($item != '.' && $item != '..') {
            if (is_dir($dir.'/'.$item)) {
                $result = remove_dir($dir.'/'.$item) && $result;
            } else {
                $result = unlink($dir.'/'.$item) && $result;
            }
        }
    }
    // Clear variables from memory...
    unset($item);
    unset($dirlist);

And it seems to work properly.

Can this change or similar (I'm not a professional PHP programmer) be included in the main Moodle code?

Regards,
Marcin


Average of ratings:Useful (1)
In reply to Marcin K.

Re: Problem with deleting directories on NFS in FreeBSD

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
That second implementation seems more prone to race conditions, if two separate requests try to manipulate the same folder.
Average of ratings: -
In reply to Tim Hunt

Re: Problem with deleting directories on NFS in FreeBSD

by Marcin K. -

Earlier in the remove_dir function we have:

    if (!$contentonly) {
        // Start by renaming the directory; this will guarantee that other processes don't write to it
        // while it is in the process of being deleted.
        $tempdir = rename_to_unused_name($dir);
        if ($tempdir) {
            // If the rename was successful then delete the $tempdir instead.
            $dir = $tempdir;
        }
        // If the rename fails, we will continue through and attempt to delete the directory
        // without renaming it since that is likely to at least delete most of the files.
    }

So if my solution is more prone to race conditions - I'm not sure, because some kind of race conditions pushed mi to make mofifications in moodlelib.php - maybe good solution is use my version in directory was renemed succesfully and original version otherwise?

Average of ratings: -
In reply to Marcin K.

Re: Problem with deleting directories on NFS in FreeBSD

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
Sorry, I had forgotten that bit.
Average of ratings: -