Bug 24749 - Plack restart should clear orphaned cgi_compile directories on restart
Summary: Plack restart should clear orphaned cgi_compile directories on restart
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-02-27 18:16 UTC by Nick Clemens (kidclamp)
Modified: 2021-07-14 16:40 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 24749: Attempt to use tmp_path from koha-conf for TMPDIR for starman (1.77 KB, patch)
2020-09-09 14:44 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2020-02-27 18:16:06 UTC
When a plack process crashes you can end up with directories left from CGI::Compile in tmp

These dirs are owned by the instance and named cgi_compile_{pid}

Later on, other instances can generate the same dir name because the pid is reused. 

If these orphaned dirs still exist then plack locks up, unable to create/access the directory

This may be a CGI::Compile bug, but if we can find a way to delete these on restart that would mitigate the issue
Comment 1 Nick Clemens (kidclamp) 2020-02-27 19:21:03 UTC
Related error code:
Error in tempfile() using template /tmp/cgi_compile_2076/cgi_compile_XXXXX.pm: Could not create temp file /tmp/cgi_compile_2076/cgi_compile_gNjo5.pm: Permission denied at /usr/share/perl5/CGI/Compile.pm line 181.
Comment 2 Nick Clemens (kidclamp) 2020-09-09 14:44:18 UTC
Created attachment 109826 [details] [review]
Bug 24749: Attempt to use tmp_path from koha-conf for TMPDIR for starman

The issue described on this bug is triggered when a process ID is reused, but the directory
belongs to another koha instance on the same server

To mitigate this issue we attemtped to force plack to use an instance specific
TMPDIR as CGI::Compile simply uses File::Spec::tmpdir to decide where to compile
files

If the folder belongs to the instance then we shouldn't have permissions issues

Sadly these patches, while brilliant, do not work

Attaching in the hopes someone even more brilliant can complete the work
Comment 3 Marcel de Rooy 2020-09-09 14:47:43 UTC
Time for containers, Nick ?
Comment 4 David Cook 2020-09-10 03:17:50 UTC
That's an interesting one.

I would say that's a CGI::Compile bug/issue. 

Looking at https://metacpan.org/release/CGI-Compile/source/lib/CGI/Compile.pm, it's sad that they use File::Temp for the temporary file but not for the temporary directory. Bizarre.

It looks like you're setting TMPDIR in the Plack middleware, but that's probably only going to be relevant to the PSGI application rather than the Plack code around it. 

I'd suggest setting TMPDIR in /etc/koha/plack.psgi. You should be able to call C4::Context->temporary_directory there.
Comment 5 David Cook 2020-09-10 03:20:24 UTC
Comment on attachment 109826 [details] [review]
Bug 24749: Attempt to use tmp_path from koha-conf for TMPDIR for starman

Review of attachment 109826 [details] [review]:
-----------------------------------------------------------------

::: Koha/Middleware/SetEnv.pm
@@ +96,5 @@
>      } @setenv_headers;
>  
> +    $setenvs{TMPDIR} = C4::Context->temporary_directory;
> +    # See bug 24749
> +

I don't think CGI::Compile will see this, as this'll be set in the PSGI application I think.

::: debian/scripts/koha-plack
@@ +84,5 @@
>      [ -z $PLACK_MAX_REQUESTS ] && PLACK_MAX_REQUESTS="50"
>      PLACK_WORKERS=$(run_safe_xmlstarlet $instancename plack_workers)
>      [ -z $PLACK_WORKERS ] && PLACK_WORKERS="2"
> +    KOHATMPDIR=$(run_safe_xmlstarlet $instancename tmp_path)
> +    [ -z $KOHATMPDIR ] || TMPDIR=$KOHATMPDIR

You need to export TMPDIR here. Otherwise, it's just going to be localized to this script. 

(But really you could just define it in plack.psgi instead, which would be a more robust solution for different deployments.)

You can see this by how KOHA_CONF and PERL5LIB are exported in koha-plack.