Hello. I discovered some problems with Dancer::MIME under Starman::Server (type Net::Server::PreFork). After some debugging i found that MIME::Types (used in Dancer::MIME) read and parse mime types from it DATA handle. This handle is opening by Perl during module load procedure. But subsequent reads occured in &MIME::Types::init (which called from constructor). In Dancer flow it occurs after fork, in different processes concurrently. Thereby some processes had invalid MIME data. I propose three different solutions: 1. Remove MIME::Types from Dancer::MIME completely :) 2. Explicitly call &MIME::Types::init early (before fork): package Dancer::MIME; use strict; use warnings; use base 'Dancer::Object::Singleton'; use MIME::Types; MIME::Types->init; 3. Use MIME::Types later (in &Dancer::MIME::init): sub init { my ($class, $instance) = @_; eval "use MIME::Types"; $instance->mime_type(MIME::Types->new(only_complete => 1)); $instance->aliases({}); } What's more properly? Thank you. -- Cheers, Oleg A. Mamontov mailto: oleg@mamontov.net jabber: lonerr@charla.mamontov.net icq uin: 79-521-617 cell: +7-903-798-1352
Hi, On 10/02/2011 15:37, Oleg A. Mamontov wrote:
After some debugging i found that MIME::Types (used in Dancer::MIME) read and parse mime types from it DATA handle. This handle is opening by Perl during module load procedure. But subsequent reads occured in&MIME::Types::init (which called from constructor).
Yes, that's my code. The problem with DATA is that if I request a rewind on the file handle I get to the beginning of the Perl file. Also, that would need to be fixed directly on the module that, probably, we do not have access.
1. Remove MIME::Types from Dancer::MIME completely :)
This would lead to the need of replicating some of the code. Although Dancer folks try to minimize the number of dependencies I try to convince them to minimize the amount of proprietary code that is already available on other modules :)
2. Explicitly call&MIME::Types::init early (before fork):
package Dancer::MIME; use strict; use warnings; use base 'Dancer::Object::Singleton'; use MIME::Types;
MIME::Types->init;
3. Use MIME::Types later (in&Dancer::MIME::init):
sub init { my ($class, $instance) = @_; eval "use MIME::Types"; $instance->mime_type(MIME::Types->new(only_complete => 1)); $instance->aliases({}); }
Did you try both solutions? Personally I prefer the first one and I'm happy to prepare the patch on git. Cheers ambs
On Feb 10, 2011, at 6:45 PM, ambs wrote:
Hi,
On 10/02/2011 15:37, Oleg A. Mamontov wrote:
After some debugging i found that MIME::Types (used in Dancer::MIME) read and parse mime types from it DATA handle. This handle is opening by Perl during module load procedure. But subsequent reads occured in&MIME::Types::init (which called from constructor).
Yes, that's my code. The problem with DATA is that if I request a rewind on the file handle I get to the beginning of the Perl file. Also, that would need to be fixed directly on the module that, probably, we do not have access. You can't rewind position pointer anyway, cause of concurrently (non-atomic) readline operations :(
1. Remove MIME::Types from Dancer::MIME completely :)
This would lead to the need of replicating some of the code. Although Dancer folks try to minimize the number of dependencies I try to convince them to minimize the amount of proprietary code that is already available on other modules :)
2. Explicitly call&MIME::Types::init early (before fork):
package Dancer::MIME; use strict; use warnings; use base 'Dancer::Object::Singleton'; use MIME::Types;
MIME::Types->init;
3. Use MIME::Types later (in&Dancer::MIME::init):
sub init { my ($class, $instance) = @_; eval "use MIME::Types"; $instance->mime_type(MIME::Types->new(only_complete => 1)); $instance->aliases({}); }
Did you try both solutions? Personally I prefer the first one and I'm happy to prepare the patch on git.
Yes and preffer the first one too :) But drawback of this solution is loss of lazy initialization. On my iMac this is + ~30ms to Dancer startup.
Cheers ambs _______________________________________________ Dancer-users mailing list Dancer-users@perldancer.org http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
-- Cheers, Oleg A. Mamontov mailto: oleg@mamontov.net jabber: lonerr@charla.mamontov.net icq uin: 79-521-617 cell: +7-903-798-1352
On 10/02/2011 16:01, Oleg A. Mamontov wrote:
2. Explicitly call&MIME::Types::init early (before fork):
package Dancer::MIME; use strict; use warnings; use base 'Dancer::Object::Singleton'; use MIME::Types;
MIME::Types->init;
3. Use MIME::Types later (in&Dancer::MIME::init):
sub init { my ($class, $instance) = @_; eval "use MIME::Types"; $instance->mime_type(MIME::Types->new(only_complete => 1)); $instance->aliases({}); }
Did you try both solutions? Personally I prefer the first one and I'm happy to prepare the patch on git.
Yes and preffer the first one too :) But drawback of this solution is loss of lazy initialization. On my iMac this is + ~30ms to Dancer startup.
Well, Dancer doesn't start that often. But, sukria, what do you say? You're the boss :D ambs
On 10/02/2011 16:20, sawyer x wrote:
On Thu, Feb 10, 2011 at 6:04 PM, ambs <ambs+dancer@perl-hackers.net <mailto:ambs%2Bdancer@perl-hackers.net>> wrote:
But, sukria, what do you say? You're the boss :D
Sukria doesn't like the title "boss" :)
We call him "the architect". :)
ok, Dear architect, how should I patch Dancer::MIME? :)
Hi, Le 10/02/2011 17:04, ambs a écrit :
On 10/02/2011 16:01, Oleg A. Mamontov wrote:
2. Explicitly call&MIME::Types::init early (before fork):
package Dancer::MIME; use strict; use warnings; use base 'Dancer::Object::Singleton'; use MIME::Types;
MIME::Types->init;
[...] Did you try both solutions? Personally I prefer the first one and I'm happy to prepare the patch on git.
Yes and preffer the first one too :) But drawback of this solution is loss of lazy initialization. On my iMac this is + ~30ms to Dancer startup.
Well, Dancer doesn't start that often.
But, sukria, what do you say? You're the boss :D
I also like that strategy. Dancer startup is not that bad and I think we can accept an extra ms in order to fix an issue. That's understandable Concerning MIME::Types though, I have another -unrelated- issue: MIME::Types brings 3 Test:: modules. We should really bug the author to make these deps dynamic, see what happens on a fresh install of Dancer, on a brand new perlbrew 5.12.2 : 2 3 4 5 6 7 8 Fetching http://search.cpan.org/CPAN/authors/id/S/SU/SUKRIA/Dancer-1.3003.tar.gz ... OK Fetching http://search.cpan.org/CPAN/authors/id/M/MI/MIYAGAWA/HTTP-Server-Simple-PSGI... ... OK Fetching http://search.cpan.org/CPAN/authors/id/J/JE/JESSE/HTTP-Server-Simple-0.43.ta... ... OK Fetching http://search.cpan.org/CPAN/authors/id/G/GE/GETTY/HTTP-Body-1.11.tar.gz ... OK Fetching http://search.cpan.org/CPAN/authors/id/R/RJ/RJBS/Test-Deep-0.108.tar.gz ... OK Fetching http://search.cpan.org/CPAN/authors/id/F/FD/FDALY/Test-Tester-0.107.tar.gz ... OK Fetching http://search.cpan.org/CPAN/authors/id/A/AD/ADAMK/Test-NoWarnings-1.02.tar.g... ... OK Fetching http://search.cpan.org/CPAN/authors/id/M/MA/MARKOV/MIME-Types-1.31.tar.gz ... OK So we have exactly 7 non-core deps under 5.12.2, with a MIME::Types updated this would be 4 (Test::Tester, Test::NoWarnings and Test::Deep are required by MIME;::Types, not Dancer itself). Of course if we decide to rewrite MIME::Types the problem disappears, but it might be a bit overkill (and source of new Dancer-unrelated bugs)... Regards, PS: I'm the one who started the project, that's true, but Dancer now lives on his own, thanks to its awesome community and dedicated core-team members, so "boss" doesn't mean anything ;-) My ego likes the term "architect", though, I must say :D -- Alexis Sukrieh
Hello all, and Sukria I offer myself to: 1. prepare the patch 2. contact MIME::Types author and check if I have luck :) All the best ambs On 10/02/2011 18:06, Alexis Sukrieh wrote:
Hi,
Le 10/02/2011 17:04, ambs a écrit :
On 10/02/2011 16:01, Oleg A. Mamontov wrote:
2. Explicitly call&MIME::Types::init early (before fork):
package Dancer::MIME; use strict; use warnings; use base 'Dancer::Object::Singleton'; use MIME::Types;
MIME::Types->init;
[...] Did you try both solutions? Personally I prefer the first one and I'm happy to prepare the patch on git.
Yes and preffer the first one too :) But drawback of this solution is loss of lazy initialization. On my iMac this is + ~30ms to Dancer startup.
Well, Dancer doesn't start that often.
But, sukria, what do you say? You're the boss :D
I also like that strategy. Dancer startup is not that bad and I think we can accept an extra ms in order to fix an issue. That's understandable
Concerning MIME::Types though, I have another -unrelated- issue: MIME::Types brings 3 Test:: modules. We should really bug the author to make these deps dynamic, see what happens on a fresh install of Dancer, on a brand new perlbrew 5.12.2 :
2 3 4 5 6 7 8
Fetching http://search.cpan.org/CPAN/authors/id/S/SU/SUKRIA/Dancer-1.3003.tar.gz ... OK Fetching http://search.cpan.org/CPAN/authors/id/M/MI/MIYAGAWA/HTTP-Server-Simple-PSGI... ... OK Fetching http://search.cpan.org/CPAN/authors/id/J/JE/JESSE/HTTP-Server-Simple-0.43.ta... ... OK Fetching http://search.cpan.org/CPAN/authors/id/G/GE/GETTY/HTTP-Body-1.11.tar.gz ... OK Fetching http://search.cpan.org/CPAN/authors/id/R/RJ/RJBS/Test-Deep-0.108.tar.gz ... OK Fetching http://search.cpan.org/CPAN/authors/id/F/FD/FDALY/Test-Tester-0.107.tar.gz ... OK Fetching http://search.cpan.org/CPAN/authors/id/A/AD/ADAMK/Test-NoWarnings-1.02.tar.g... ... OK Fetching http://search.cpan.org/CPAN/authors/id/M/MA/MARKOV/MIME-Types-1.31.tar.gz ... OK
So we have exactly 7 non-core deps under 5.12.2, with a MIME::Types updated this would be 4 (Test::Tester, Test::NoWarnings and Test::Deep are required by MIME;::Types, not Dancer itself).
Of course if we decide to rewrite MIME::Types the problem disappears, but it might be a bit overkill (and source of new Dancer-unrelated bugs)...
Regards,
PS: I'm the one who started the project, that's true, but Dancer now lives on his own, thanks to its awesome community and dedicated core-team members, so "boss" doesn't mean anything ;-)
My ego likes the term "architect", though, I must say :D
On Thu, Feb 10, 2011 at 4:37 PM, Oleg A. Mamontov <oleg@mamontov.net> wrote:
After some debugging i found that MIME::Types (used in Dancer::MIME) read and parse mime types from it DATA handle. This handle is opening by Perl during module load procedure. But subsequent reads occured in &MIME::Types::init (which called from constructor).
...
I propose three different solutions:
1. Remove MIME::Types from Dancer::MIME completely :)
2. Explicitly call &MIME::Types::init early (before fork):
package Dancer::MIME; use strict; use warnings; use base 'Dancer::Object::Singleton'; use MIME::Types;
MIME::Types->init;
3. Use MIME::Types later (in &Dancer::MIME::init):
sub init { my ($class, $instance) = @_; eval "use MIME::Types"; $instance->mime_type(MIME::Types->new(only_complete => 1)); $instance->aliases({}); }
I would dare to propose two additional potential solutions: 4. Solve the issue in MIME::Type, the author is usually very responsive and he should be willing to accept patches 5. Investigate the possibility to use Media::Type::Simple, from the feedback page of MIME::Type it seems that this module does the same things of MIME::Type but with additional features. I can try to propose a patch for MIME::Type to MARKOV (i.e. start working on solution 4). Cheers, Flavio.
Hi guys, As reported by Chris, I think this whole thread is linked to issue GH#136. This issues was fixed, but the fix was then removed accidentally by an other commit. Chris has proposed to reintroduce the fix ( basically just BEGIN { MIME::Types->new(only_complete => 1); } ) He has also provided a test to check it. Please check his PR#310 ( https://github.com/sukria/Dancer/pull/310/ ) I think that's all we need for now, but don't take my word for it :) Have a look at it, and possibly propose additional changes, or a better solution. In any case, it'd be good to gather the efforts on this fix Thanks On 10 February 2011 19:25, Flavio Poletti <polettix@gmail.com> wrote:
On Thu, Feb 10, 2011 at 4:37 PM, Oleg A. Mamontov <oleg@mamontov.net> wrote:
After some debugging i found that MIME::Types (used in Dancer::MIME) read and parse mime types from it DATA handle. This handle is opening by Perl during module load procedure. But subsequent reads occured in &MIME::Types::init (which called from constructor).
...
I propose three different solutions:
1. Remove MIME::Types from Dancer::MIME completely :)
2. Explicitly call &MIME::Types::init early (before fork):
package Dancer::MIME; use strict; use warnings; use base 'Dancer::Object::Singleton'; use MIME::Types;
MIME::Types->init;
3. Use MIME::Types later (in &Dancer::MIME::init):
sub init { my ($class, $instance) = @_; eval "use MIME::Types"; $instance->mime_type(MIME::Types->new(only_complete => 1)); $instance->aliases({}); }
I would dare to propose two additional potential solutions: 4. Solve the issue in MIME::Type, the author is usually very responsive and he should be willing to accept patches 5. Investigate the possibility to use Media::Type::Simple, from the feedback page of MIME::Type it seems that this module does the same things of MIME::Type but with additional features.
I can try to propose a patch for MIME::Type to MARKOV (i.e. start working on solution 4). Cheers, Flavio. _______________________________________________ Dancer-users mailing list Dancer-users@perldancer.org http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
Please retry with latest Dancer from devel branch, I've applied Chris fix. Granted, it's a workaround, but it looks like it's not a bug in MIME::Types, but in the preforking platforms. Hard to fix anyway. On 11 February 2011 14:40, damien krotkine <dkrotkine@gmail.com> wrote:
Hi guys,
As reported by Chris, I think this whole thread is linked to issue GH#136. This issues was fixed, but the fix was then removed accidentally by an other commit.
Chris has proposed to reintroduce the fix ( basically just BEGIN { MIME::Types->new(only_complete => 1); } )
He has also provided a test to check it.
Please check his PR#310 ( https://github.com/sukria/Dancer/pull/310/ )
I think that's all we need for now, but don't take my word for it :) Have a look at it, and possibly propose additional changes, or a better solution.
In any case, it'd be good to gather the efforts on this fix
Thanks
On 10 February 2011 19:25, Flavio Poletti <polettix@gmail.com> wrote:
On Thu, Feb 10, 2011 at 4:37 PM, Oleg A. Mamontov <oleg@mamontov.net> wrote:
After some debugging i found that MIME::Types (used in Dancer::MIME) read and parse mime types from it DATA handle. This handle is opening by Perl during module load procedure. But subsequent reads occured in &MIME::Types::init (which called from constructor).
...
I propose three different solutions:
1. Remove MIME::Types from Dancer::MIME completely :)
2. Explicitly call &MIME::Types::init early (before fork):
package Dancer::MIME; use strict; use warnings; use base 'Dancer::Object::Singleton'; use MIME::Types;
MIME::Types->init;
3. Use MIME::Types later (in &Dancer::MIME::init):
sub init { my ($class, $instance) = @_; eval "use MIME::Types"; $instance->mime_type(MIME::Types->new(only_complete => 1)); $instance->aliases({}); }
I would dare to propose two additional potential solutions: 4. Solve the issue in MIME::Type, the author is usually very responsive and he should be willing to accept patches 5. Investigate the possibility to use Media::Type::Simple, from the feedback page of MIME::Type it seems that this module does the same things of MIME::Type but with additional features.
I can try to propose a patch for MIME::Type to MARKOV (i.e. start working on solution 4). Cheers, Flavio. _______________________________________________ Dancer-users mailing list Dancer-users@perldancer.org http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
participants (8)
-
Alberto Simoes -
Alexis Sukrieh -
ambs -
damien krotkine -
Flavio Poletti -
Oleg A. Mamontov -
Puneet Kishor -
sawyer x