Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

Trap SIGPIPE on Travis #1040

Closed
wants to merge 42 commits into from
Closed

Trap SIGPIPE on Travis #1040

wants to merge 42 commits into from

Conversation

fauvel
Copy link
Contributor

@fauvel fauvel commented Feb 28, 2014

No description provided.

- Happening on shutdown, we're losing the PDO connection while the code coverage is running
- Should work fine now in PHP 5.4
@njam
Copy link
Member

njam commented Feb 28, 2014

  • can we use SIGPIPE instead of 13 for readability?
  • Do we need absolute path to php? Or could we use just php?
  • Instead of empty trap do something like # this happens because of...?

@fauvel
Copy link
Contributor Author

fauvel commented Feb 28, 2014

It looks like I'm still missing some elements to reproduce this green build...

@fauvel
Copy link
Contributor Author

fauvel commented Feb 28, 2014

Interesting :)

@fauvel
Copy link
Contributor Author

fauvel commented Feb 28, 2014

Find the difference: 26be461...70da096...

@fauvel
Copy link
Contributor Author

fauvel commented Mar 3, 2014

I'm pretty sure now it is an issue with the PDO connection opened by the AWS SDK.

@njam
Copy link
Member

njam commented Mar 3, 2014

Hmmm, why is the AWS SDK opening a PDO connection? We use the "s3" API only, I don't think it should (nor is configured) to open a database connection.

@fauvel
Copy link
Contributor Author

fauvel commented Mar 3, 2014

I'm not absolutely sure, but gdb said we have a SEGPIPE on shutdown as a static class member variable is being freed and tries to close a PDO connection... My last suspect is CachePDO::$pdo, in vendor/amazonwebservices/aws-sdk-for-php/lib/cachecore/cachepdo.class.php. But it's not even static...

@njam
Copy link
Member

njam commented Mar 3, 2014

Maybe try disabling the AWS-related tests and see whether it works?

@fauvel
Copy link
Contributor Author

fauvel commented Mar 3, 2014

Yes, it does works without AWS-related tests.

But even CM_Action_AbstractTest::testNotify() triggers a Segfault in the end...

So maybe the problem rather comes from phpredis? CM_Action_Abstract::_track() instantiates a Redis object via CM_KissTracking::track(), CM_KissTracking::_getSet() and CM_SetAdapter_Redis::add(). And there we have a connection (using PDO internally?) that we're not closing explicitly. And it is being stored statically on CM_Redis_Client::getInstance(), which would explain the gdb backtrace.

@njam
Copy link
Member

njam commented Mar 3, 2014

Hmm I don't think the redis extension uses PDO code.
Hmhmhm

@fauvel
Copy link
Contributor Author

fauvel commented Mar 3, 2014

Is it just me, or is there a problem with the linking of Travis builds right now? I can see an indicator besides the commit ssh on GitHub as long as the build is running, but then it disappears...

@njam
Copy link
Member

njam commented Mar 3, 2014

Mhhm had it before. Maybe make the PR mergeable, maybe it helps..?

Conflicts:
	tests/library/CM/Action/AbstractTest.php
	tests/library/CM/KissTrackingTest.php
@fauvel
Copy link
Contributor Author

fauvel commented Mar 3, 2014

So it did pass at e08237e, with CM_KissTrackingTest disabled and all AWS-related tests enabled.

@njam
Copy link
Member

njam commented Mar 3, 2014

So this aws sdk is actually long deprecated - maybe we should try upgrading to the new version?

@fauvel
Copy link
Contributor Author

fauvel commented Mar 4, 2014

Closed in favor of #1046

@scrutinizer-notifier
Copy link

The inspection completed: No new issues

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants