Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #460: Locking session while saving large content #461

Open
wants to merge 1 commit into
base: stable
Choose a base branch
from

Conversation

katerynadegtyariova
Copy link

The description of the problem and steps to repeat are provided in Issue #460.

Profiling shows that 95% of time is spent inside h5p.classes.php: H5PExport > createExportFile > $zip->Close() call:

https://github.com/h5p/h5p-php-library/blob/c84217f8414c06ebabe3dedb046a77c42c2f0dbf/h5p.classes.php#L1931

However, the library submodule is used not only with Moodle. For this reason the patch is proposed for Moodle plugin.

The solution for the session locking would be to write_close() the session before finishing zip file. It will allow other pages to use session without waiting on the file operation to finish.

The main concern is that framework::messages() should not be called before the session is closed.
My testing confirms that framework::messages() is not called at all when the file content is being saved.

public static function messages($type, $newmessage = null, $code = null) {
static $m = 'mod_hvp_messages';
if ($newmessage === null) {
// Return and reset messages.
$messages = isset($_SESSION[$m][$type]) ? $_SESSION[$m][$type] : array();
unset($_SESSION[$m][$type]);
if (empty($_SESSION[$m])) {
unset($_SESSION[$m]);
}

framework::messages() is only called when the content is loaded for viewing or editing. However, in such case the session is not closed (the session will be closed only before the file storing).

The testing method is described in Issue #460. Although the file saving can take considerable time (which is expected for large files), the session is not locked and another page can be opened in a separate tab without giving errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant