Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
standUpServer
is called by visit
, and even though we have a really nice comment:
// If there's not a server at this point, make one.
that would make you think we only start a new server conditionally, we seem to always start a new server. On every single visit.
This leaves dangling file locks and eats up server processes because we leave lots of hanging php servers that aren't ever stopped.
Proposed resolution
Do something like:
// If there's not a server at this point, make one.
if (!$this->serverProcess || $this->serverProcess->isTerminated()) {
$this->serverProcess = $this->instantiateServer($this->getPortNumber(), $working_dir);
if ($this->serverProcess) {
$this->serverDocroot = $working_dir;
}
}
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#11 | interdiff.txt | 532 bytes | Mile23 |
#11 | 3088447_11.patch | 2.37 KB | Mile23 |
#7 | interdiff.txt | 865 bytes | Mile23 |
#7 | 3088447_7.patch | 2.37 KB | Mile23 |
#7 | 3088447_7_test_only.patch | 1.49 KB | Mile23 |
Comments
Comment #2
heddnComment #3
alexpottIs this testable? Also it looks like the comment above needs an update.
Comment #4
heddnIt isn't really testable. Not sure the comment needs update, unless I'm missing something. Rather the new code is fulfilling the promise of the comment now.
Comment #5
Mile23Adds a test.
Comment #6
heddnI can confirm the failure test works:
Comment #7
Mile23This always seemed awkward, so I fixed it.
Also, here's a test-only patch as well as the full one.
Comment #9
heddnI wish I could RTBC this. I wrote the code, Paul wrote the test. I'm good w/ the test. If he's good with the code under test, can we mark this RTBC jointly ;)
Comment #10
Krzysztof DomańskiLooks realy good! RTBC+1
Minor CS: expected 1 blank line after function.
Comment #11
Mile23Addressed CS issue.
Comment #12
Krzysztof DomańskiThanks!
Comment #13
alexpottCommitted and pushed eecc0efc4e to 9.0.x and b02aad0305 to 8.9.x. Thanks!
Comment #16
alexpottDiscussed with @catch we agreed this was good to backport to 8.8.x