Problem/Motivation

While working on #2568171: Upgrade to Twig 1.22 and implement our own cache class, @znerol was able to file a pull request upstream to fix the race condition upstream in Twig, so we no longer need to override loadTemplate to handle the race condition ourselves.

This also means we have less work to do in #2568181: [meta] Get ready to upgrade to Twig 2.x.

Proposed resolution

Remove our override and update our cache class to match the new interface (has() has been removed).

Remaining tasks

TBD

User interface changes

n/a

API changes

Nothing relevant.

Data model changes

None

Files: 
CommentFileSizeAuthor
#9 remove_race_condition-2571817-9.patch17.79 KBhussainweb
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,909 pass(es). View
#9 interdiff-2-9.txt5.68 KBhussainweb
#2 remove_race_condition-2571817-2-test.patch17.18 KBCottser
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,578 pass(es). View
#2 remove_race_condition-2571817-2.patch2.61 KBCottser
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 64,779 pass(es), 17,273 fail(s), and 7,178 exception(s). View

Comments

Cottser created an issue. See original summary.

Cottser’s picture

Title: Remove our race handling in TwigEnvironment::loadTemplate(), it's fixed upstream now » Remove race condition handling in TwigEnvironment::loadTemplate(), it's fixed upstream now
Status: Active » Needs review
FileSize
2.61 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 64,779 pass(es), 17,273 fail(s), and 7,178 exception(s). View
17.18 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,578 pass(es). View

We need to wait until the next Twig release to do something like this and use ~1.22.2 in our core/composer.json.

Cottser’s picture

Status: Needs review » Postponed

The last submitted patch, 2: remove_race_condition-2571817-2.patch, failed testing.

The last submitted patch, 2: remove_race_condition-2571817-2.patch, failed testing.

Cottser’s picture

Issue summary: View changes
Cottser’s picture

Assigned: Cottser » Unassigned
Cottser’s picture

Status: Postponed » Needs work

Working on this now.

hussainweb’s picture

Status: Needs work » Needs review
FileSize
5.68 KB
17.79 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,909 pass(es). View

I used the -test patch from #2 and updated the version constraint to ^1.22.2 (so that it picks up everything until 2.0). I think that was the intention, right?

Cottser’s picture

Yeah that's fine. A comment would have been nice (or ping or something), I was about to post a patch. Anyway, thanks.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Nice!

hussainweb’s picture

@Cottser, I read your comment again. For some reason I thought you said needs work now. Sorry for the confusion.

Cottser’s picture

@hussainweb no worries it's been a long day! We can hug it out if you are at Barcelona.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed dc95ff1 and pushed to 8.0.x. Thanks!

  • alexpott committed dc95ff1 on 8.0.x
    Issue #2571817 by Cottser, hussainweb: Remove race condition handling in...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.