wierd APC error error: require_once(): Cannot redeclare class insertquery_mysql in .

[eileenmcnaughton]

We have been hitting the above error using IATS. This appears to be something we just don't want to know about - see

http://help.getpantheon.com/pantheon/topics/fatal_error_require_once_cannot_redeclare_class_insertquery_mysql

One punter says that "The real solution is to remove the Call-time pass-by-reference from your code. "

However, I found that I could FIX it (make it go away) by removing one line of code from IATS.

Now, I happen to think this line of code is not required - it seems to have spawned itself all over the CiviCRM code-base more like a virus than enzyme.

This is the dreaded line...
// get merchant data from config
$config = CRM_Core_Config::singleton();

Now, since we aren't using $config we know we don't need the variable. But, do we need to call CRM_Core_Config::singleton() at all?

My understanding is that line IS overloaded and does instantiate 'something' @totten might know.

But I'm pretty comfortable arguing the something isn't required here because
a) I find other processors without it &
b) you don't get to constructing a payment processor object without having core pretty heavily loaded.

Sending a PR to remove the line

[totten]

I've bumped into CRM_Core_Config::singleton() in the context of civicrm/civicrm-core#5114 (which is a long-term WIP; notice that the second commit replaces many calls to it). It's part of the current/old bootstrap protocol -- after loading settings, one calls Config::singleton(). I believe this ensures some core resources (civicrm_domain.backend and data-folders) are initialized, but I haven't really dug-in to figure out the history or full rationale. #5114 is written with an aim to incorporate Config::singleton(), make it optional, and eventually remove.

Another note in support of removing: the comment before it says "get merchant data from config" -- but that data should already be injected via $paymentProcessor. This line smells like a left-over from some old logic. However, it is possible that it was added to deal with some bug (an extremely quirky, indirect, hard-to-reproduce one?), so you might want dig through "git blame/svn blame" and ask the original author.

[xurizaemon]

My understanding is that line IS overloaded and does instantiate 'something' @totten might know.

If there are side-effects from that call, they are undocumented and I call them a bug. https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/Config.php doesn't state that there are any side-effects in the documentation.

As far as call-time pass-by-reference goes, I see these candidates in fuzionnz/civicrm-core @ 4.4.13rc1 (which I think is what's running on CANY).

CRM/Utils/Tree.php
138:      if ($node = &$this->findNode($name, $cNode)) {
209:    if ($parentNode = &$this->findNode($parentName, $temp)) {
212:        if ($cNode = &$this->findNode($childName, $parentNode)) {

These remain in civicrm/civicrm-core @ master.

[xurizaemon]

PS. I don't think @totten will get notified of mentions unless he's got access to this repo. I'm not sure what Github does in that case.

Oh. He turned up. Never mind that then! I learned something ... that this issue is in the iATS repo, not in the client's private repo, and a reminder to check if there are new comments when you've been on a call for an hour. 🐑

[eileenmcnaughton]

Hmm the apc error 'came back' - not sure the approach at the moment. Still think the PR is correct though

[xurizaemon]

👍