Ensure run() always returns Application instance #3648

Merged
merged 1 commit into from Feb 1, 2013

Conversation

Projects
None yet
2 participants
@weierophinney
Member

weierophinney commented Feb 1, 2013

  • Some paths through Application::run() would call
    return $this->completeRequest($event); this particular method was returning
    a response object, which meant that send() was resulting in a double-render.
    The main run() routine typically returns the Application instance, where
    send() is a no-op, and thus no double-render can occur.
  • The workaround until this is merged and released is to remove the call to
    ->send() in your public/index.php.

This fixes a situtation reported by Demian Katz on the contributors mailing list.

Ensure run() always returns Application instance
- Some paths through `Application::run()` would call
  `return $this->completeRequest($event)`; this particular method was returning
  a response object, which meant that `send()` was resulting in a double-render.
  The main `run()` routine typically returns the Application instance, where
  `send()` is a no-op, and thus no double-render can occur.
- The workaround until this is merged and released is to remove the call to
  `->send()` in your `public/index.php`.

@Maks3w Maks3w merged commit 643a99e into zendframework:master Feb 1, 2013

1 check passed

default The Travis build passed
Details
@Maks3w

This comment has been minimized.

Show comment Hide comment
@Maks3w

Maks3w Feb 1, 2013

Member

This is throwing "Segment Fault" in Travis tests. Unfortunately due recent changes in Travis the global output don't reflect this.

Member

Maks3w commented Feb 1, 2013

This is throwing "Segment Fault" in Travis tests. Unfortunately due recent changes in Travis the global output don't reflect this.

@weierophinney

This comment has been minimized.

Show comment Hide comment
@weierophinney

weierophinney Feb 1, 2013

Member

The segfault only appears to be occurring in PHP 5.3.3, and based on the number of tests run (I count 174), the next test should be RestfulControllerTest::testMethodOverloadingShouldReturnPluginWhenFound -- which isn't even part of this commit.

I've run the tests locally using PHP 5.3.3 and PHPUnit 3.7.13 (the same versions as we see on Travis), and cannot reproduce. I'm unsure what to do at this point.

Member

weierophinney commented Feb 1, 2013

The segfault only appears to be occurring in PHP 5.3.3, and based on the number of tests run (I count 174), the next test should be RestfulControllerTest::testMethodOverloadingShouldReturnPluginWhenFound -- which isn't even part of this commit.

I've run the tests locally using PHP 5.3.3 and PHPUnit 3.7.13 (the same versions as we see on Travis), and cannot reproduce. I'm unsure what to do at this point.

@weierophinney

This comment has been minimized.

Show comment Hide comment
@weierophinney

weierophinney Feb 1, 2013

Member

I've restarted the job to see if it's reproducible on travis.

Member

weierophinney commented Feb 1, 2013

I've restarted the job to see if it's reproducible on travis.

@weierophinney

This comment has been minimized.

Show comment Hide comment
@weierophinney

weierophinney Feb 1, 2013

Member

Travis can reproduce the error. @Maks3w has indicated he gets the segfault on Windows with xdebug versions 2.1.4 and 2.2.1, but that the segfault does not occur without xdebug.

I cannot reproduce with either xdebug 2.1.4 or 2.2.1 on ubuntu.

Member

weierophinney commented Feb 1, 2013

Travis can reproduce the error. @Maks3w has indicated he gets the segfault on Windows with xdebug versions 2.1.4 and 2.2.1, but that the segfault does not occur without xdebug.

I cannot reproduce with either xdebug 2.1.4 or 2.2.1 on ubuntu.

@weierophinney

This comment has been minimized.

Show comment Hide comment
@weierophinney

weierophinney Feb 1, 2013

Member

The failures are not being reported in more current builds, so I think we're good now.

Member

weierophinney commented Feb 1, 2013

The failures are not being reported in more current builds, so I think we're good now.

@weierophinney weierophinney deleted the weierophinney:hotfix/application-complete-request branch Feb 1, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment