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

Embeddedrespondfactory threading issue #16

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

jwmach1
Copy link

@jwmach1 jwmach1 commented Feb 14, 2012

When Guice Stage.PRODUCTION, and more than one request was being served by the same PageTuple, they ended up sharing the same StringBuilderRespond -- kaboom!

so I moved the private final Respond respond = new StringBuilderRespond(new Object()); declaration of EmbeddedRespondFactory into the get method. Hopefully this pull request reflects that one small change to EmbeddedRespondFactory.

James and others added 17 commits November 15, 2011 21:48
…stom status codes and the other goodies of Reply
…tifactory

* create VelocityEngineProvider to create just one engine for all template compilations
…ethod interceptors (DecorateWidget change)
…eTuple, (2) that EmbedWidget has only one EmbeddedRespondFactory and (3) the EmbeddedRespondFactory was keeping a final StringBuilderRespond object to pass as the delegate to the factoried EmbeddedRespond. So, if two requests come in for the same page, they end up sharing a StringBuilderRespond
@dhanji
Copy link
Owner

dhanji commented Feb 14, 2012

Holy crap, this should never happen--thanks let me chase it down!

@jwmach1
Copy link
Author

jwmach1 commented Feb 14, 2012

This pull request is messed up because I branched from the wrong spot. sorry. The next one will be better :)

@dhanji
Copy link
Owner

dhanji commented Feb 14, 2012

Hehe, OK, thanks!

@dhanji
Copy link
Owner

dhanji commented Apr 20, 2012

Do you have a fix for this yet?

@jwmach1
Copy link
Author

jwmach1 commented Apr 23, 2012

I never got this pull request cleaned up. The Fix is changing one file --
EmbeddedRespondFactory like this

https://github.com/jwmach1/sitebricks/blob/master/sitebricks/src/main/java/com/google/sitebricks/rendering/control/EmbeddedRespondFactory.java

I did not see any negative side effects to instantiating a new new
StringBuilderRespond(new Object()); for each EmbeddedRespond.

-James

On Fri, Apr 20, 2012 at 6:57 PM, dhanji <
[email protected]

wrote:

Do you have a fix for this yet?


Reply to this email directly or view it on GitHub:
#16 (comment)

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.

2 participants