Dancer is not compatible with Corona

[berekuk]

But it recommends it as a deployment option.

How to reproduce: https://gist.github.com/berekuk/5657189

Result I observed from this gist: thread1 prints "bar…bar…bar…" until I run the thread2 in another console, and then thread1 starts printing "foo…foo…", while thread2 prints "bar…bar…bar…"

PS: It's reproducible both with Dancer and Dancer2.

[yanick]

Okay, that is funky...

I'll have to look at it a mite closer. But as far as I know, Dancer doesn't do anything special with any of the psgi servers. So I just wonder how much Corona itself is resilient to random acts of Coro'ing done within the application...

But yeah, real intriguing. Thanks for the report!

Joy, `/anick

[tehmoth]

Equivalent code using Web::Simple under Corona does the right thing. Kelp seems to fail worse though.

This gist: https://gist.github.com/tehmoth/99ed569b26e0296ddcbf (run with plackup -s Corona app.pl when connected to with just GET hello/foo: while true; do wget -q -O - 'http://localhost:5000/hello/foo';echo; done You get lines like /hello/foo:/hello/foo:foo:foo:8812944544:8812944544:8662921496:8662921496 /hello/foo:/hello/foo:foo:foo:8784741688:8784741688:8758651848:8758651848 /hello/foo:/hello/foo:foo:foo:8812944232:8812944232:8662922792:8662922792 Running GET /hello/bar at the same time causes the first script (foo) to return lines like: /hello/foo:/hello/bar:foo:bar:8662921304:8702733328:8758649928:8662921592 /hello/foo:/hello/bar:foo:bar:8617719792:8662239616:9256666880:8678678616 /hello/foo:/hello/bar:foo:bar:8617718352:8758649712:8758652112:8662924304

and the other command (bar): /hello/bar:/hello/foo:bar:foo:8702733328:8617719792:8662921592:9256666880 /hello/bar:/hello/foo:bar:foo:8662239616:8617718352:8678678616:8758652112 /hello/bar:/hello/foo:bar:foo:8758649712:8617720224:8662924304:8833652048

Notice how it seems to have re-used the request() (and the params()) object

[berekuk]

@yanick, it's not really that surprising. Coro threads share everything by default, and Dancer APIs are not object-oriented, and so don't have any means to track the right context when you enter two routes at the same time.

See: https://gist.github.com/berekuk/5661492

The only solution I can see is to track multiple Dancer::SharedData instances in a hashref, identified by Coro's thread_id. It's either this or change the entire API to the explicit $context->param(...), $context->var(...), etc. Or, give up on Coro support and change the docs :)

[tehmoth]

Berekuk: I thought one of the points of Dancer2 was the removal of Shared Data. It should be possible for functions like request and params to access data local to the current scope without relying on Coro thread_ids or converting to an explicitly object oriented API.

[berekuk]

I don't see how that's possible. I looked into Dancer2 code, and it looks like request is stored in app->context->request, so it's still one request per app, and you usually have only one app.

How do you suggest param() to access the right context, when it's just a function and not a method? If it were a closure, I could see how it could refer to the private, per-request variable, but it's not, it's exported just once when you start your app.

[yanick]

@berekuk Does the caveat given at 63deafe sounds sane to you?

[berekuk]

I'd prefer more strong wording, I think. More like "Don't even think about using Corona or any other AnyEvent-based server". In all caps, MST-style.

"Can cause" implies that there's a chance it'll be ok. So someone can decide that their app is safe, because they checked it in the browser and it looked like it's working. But it really, really won't. This issue will come up months later in production, and it will be hard to reproduce. In the worst case, users will get the personal data from other users, something they were never meant to see (so this can turn into a big security issue).

Maybe Dancer should just detect AnyEvent presence and die with fatal error.

[yanick]

"Can cause" implies that there's a chance it'll be ok.

The way I understand it, it will be okay as long as the application itself doesn't use any AnyEvent sleeps like the one you have in the gist. If the user's code don't explicitly use AnyEvent or Coro, a request can't yield to another one until it's done, and thus everything should be fine. Or am I missing one of the problematic cases?

[sukria]

sent on the way, short email! Le 28 mai 2013 13:18, "Vyacheslav Matyukhin" notifications@github.com a écrit :

I don't see how that's possible. I looked into Dancer2 code, and it looks like request is stored in app->context->request, so it's still one request per app, and you usually have only one app.

This is wrong. Dancer 2 creates one context object per request.

So Dancer 2 should behave nicely here and was designed exactly to fix that.

I suggest doing the same tests with a dancer 2 app.

How do you suggest param() to access the right context, when it's just a function and not a method? If it were a closure, I could see how it could refer to the private, per-request variable, but it's not, it's exported just once when you start your app.

You are describing here the major difference between 1 and 2.

Anything related to D1 globals issues should point to D2.

This is the case here.

[berekuk]

@yanick, you're right, but there's no reason to using Corona in this case! Also, it's possible to unintentionally yield by using a module which uses Coro internally.

@sukria, but I did, the test case is right there in my initial issue description.

[sukria]

My bad, then could you report a Bug (it is a bug) against Dancer2's repository?

Thanks.

This can't work properly on D1, but should on D2.

[ambs]

@sukria , when you say it can't work properly on D1, means we should close this issue, or that it should work badly with D1? :)

[yanick]

@ambs It means we should document the limitations of D1 w/ Corona in the POD, and close the issue. I'm working on the right wording for the documentation as we speak. :-)

[yanick]

Documentation changed with stern warning. :-)