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.
When I saved a poll in a D7 b3 site created by Aegir on Ubuntu Server 10.04, I got an error shown in the attached screenshot. When I checked the watchdog table, I copied this line from Sequel Pro:
217 2 php %type: !message in %function (line %line of %file). a:6:{s:5:"%type";s:12:"PDOException";s:8:"!message";s:49:"You cannot serialize or unserialize PDO instances";s:9:"%function";s:26:"DrupalDatabaseCache->set()";s:5:"%file";s:42:"/var/aegir/drupal-7.0b3/includes/cache.inc";s:5:"%line";i:422;s:14:"severity_level";i:3;} 3 http://www.b3.d7.joshuaoldenburg.com/ 192.168.1.4 1290270135
This is the output of php -v:
PHP 5.3.2-1ubuntu4.5 with Suhosin-Patch (cli) (built: Sep 17 2010 13:41:55)
Copyright (c) 1997-2009 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2010 Zend Technologies
I updated MySQL to the latest version a few minutes before this happened, but I think that this is a bug in PHP PDO, not MySQL. I get the error on every page. I have caching enabled, including page, block, views, and css/js aggregation. This was the first thing that I did after running apt-get update; apt-get upgrade.
Comment | File | Size | Author |
---|---|---|---|
#53 | serialize_magic.patch | 4.82 KB | dmitrig01 |
#52 | serialize_magic.patch | 4.8 KB | dmitrig01 |
#48 | serialize_magic.patch | 4.8 KB | dmitrig01 |
#45 | serialize_magic.patch | 4.8 KB | dmitrig01 |
#37 | serialize_almost_no_magic.patch | 4.62 KB | dmitrig01 |
Comments
Comment #1
Josh The Geek CreditAttribution: Josh The Geek commentedI found the function that caused this:
Also, I updated this:
mysql-client-5.1 mysql-client-core-5.1 mysql-common mysql-server mysql-server-5.1 mysql-server-core-5.1
Comment #2
Josh The Geek CreditAttribution: Josh The Geek commentedWrong priority.
The line
$fields['data'] = serialize($data); /* ??? What calls this function? */
is what causes the error. The error is not displayed on the admin pages. The site listed ('www.b3.d7.joshuaoldenburg.com') is running on a server in my closet, running Aegir on Ubuntu 10.04.1 Server.
If someone could add the data to the DB without serializing the $data object, it would (probably) fix the problem.
(BTW: It doesn't matter if I was saving a poll, or doing anything else. Saving a poll was the first thing that I did after upgrading MySQL.)
Comment #3
steaders CreditAttribution: steaders commentedI am also having the same problem on my local machine. The problem goes away once I disable the Poll module but returns if I re-enable it.
Comment #4
Josh The Geek CreditAttribution: Josh The Geek commented@steaders I can't disable the poll module. I get http://skit.ch/bymd when I view the front page, and http://skit.ch/bymr on admin/modules.
Comment #5
Josh The Geek CreditAttribution: Josh The Geek commentedFrom IRC:
12:20:06 PM chx: so $fields['data'] = serialize($data); here add this
12:20:12 PM JoshTheGeek: ok
Tykling [tykling@er.tyk.nu] entered the room. (12:20:14 PM)
MFawzy [~fawzy@41.34.233.212] entered the room. (12:20:20 PM)
afo0l_ left the room (quit: Ping timeout: 272 seconds). (12:20:29 PM)
12:20:31 PM chx: if (is_object($data) && get_class($data) != 'stdClass') var_dump(debug_backtrace());
12:20:45 PM chx: or just debug_print_backtrace
12:20:45 PM JoshTheGeek: one minute
12:20:57 PM chx: we need to see what tries to cache a PDO instance
The output was too much for FireFox or Paste Bin, so I emailed it to chx on request.
Comment #6
Josh The Geek CreditAttribution: Josh The Geek commentedI attached a zip of the output.
Comment #7
bfroehle CreditAttribution: bfroehle commentedMaybe we're trying to serialize an exception in a function which takes a PDO instance as a variable?
Anyway, here's a bit of parsing of #6:
Comment #8
bfroehle CreditAttribution: bfroehle commentedMaybe it's because we're caching the 'poll_view_voting' form which contains the following fields:
and Views is adding some sort of database objects to
$node
?Comment #9
bfroehle CreditAttribution: bfroehle commentedI have no idea if this will fix the problem, but it seems like it might work. Essentially I change poll.module to store only $node->nid and $node->title (rather than all of $node) in $form. I'm suspecting that serializing $node is causing issues when used in conjunction with Views.
Comment #10
Josh The Geek CreditAttribution: Josh The Geek commentedI'll test this when I get the chance. I haven't seen if RC1 still has the problem.
Comment #11
chx CreditAttribution: chx commentedThis is a Views issue unless someone reproduces it without Views. The original reporter did not report he had Views on and so I could not ask to test without.
Comment #12
merlinofchaos CreditAttribution: merlinofchaos commentedThe only relevance to Views in here is that view is calling node_view().
If it's even possible that Views can add something to whatever is being cached that is causing this error, it's still a bug in core that it crashes fatally. That said, I see no evidence whatsoever that Views is doing anything actually related to this bug.
Just having Views on your site when a bug happens does not make it Views' problem, chx. This is clearly a problem with core polls and form caching.
Most likely what's going on is that the view object is being added to the node during the render process here:
And then the node is being cached by the poll voting form.
It is absolutely valid to set $node->view so that the node template and preprocess functions can know about the view that is being used to render the nodes.
Comment #13
palik CreditAttribution: palik commentedsubscribing,
On my site (i use D7RC1 + Views) it looks like this:
i post 2 polls on front page, they show ok,
i make a view that shows all published polls as nodes in a block - even on prevew it gives me this error,
i put this block on main page - i got error on main pagem,
i turn off this block and main page is ok
i make a view that shows all published polls as html list with node-title field - it is ok,
Comment #14
bfroehle CreditAttribution: bfroehle commentedSetting to needs work, since that's what this issue needs.
Comment #15
Blooniverse CreditAttribution: Blooniverse commented... I can confirm this strange behaviour, as you can see on #991604: PDOException error: creation of one poll node kills whole frontpage. I am asking myself why PDO instead of the simple, intuitive ADODB (but this is a different chapter, isn't it).
Comment #16
Josh The Geek CreditAttribution: Josh The Geek commented@chx: I use Views. I have a views block that shows two most recent polls.
@palik: That's what I have set up. I think that's the problem.
Comment #17
Damien Tournoud CreditAttribution: Damien Tournoud commentedIt is exactly it. That means it's a Views bug: Views makes the $node object not serializable.
Comment #18
bfroehle CreditAttribution: bfroehle commentedOr perhaps it is a problem with the poll module --- do we really need to serialize ALL of
$node
?Comment #19
merlinofchaos CreditAttribution: merlinofchaos commentedFine then.
It was never a problem for the $view to add itself to a node in D6. This is a regression from D7. If core doesn't want to fix it, that's fine. But I am not changing it in Views.
Frankly, I think the core behavior is wrong, and kicking this issue over to Views, again, is childish and wrong. But hey, if core wants to have stupid design decisions, that's core's problem.
I will simply document in the Views help that you can't view poll nodes with Views because core behavior is lame.
Comment #20
Damien Tournoud CreditAttribution: Damien Tournoud commentedThere is absolutely no change of behavior, and as a consequence, no regression from core here. The poll module has always been serializing $node, and it's not the only place that we do it.
It's Views that, by attaching itself to $node and exposing the PDOStatement object, that prevents $node from being serialized.
If you want to keep stupid design decisions, that's Views' problem, but the current behavior seems lame in all regards.
Comment #21
Josh The Geek CreditAttribution: Josh The Geek commentedBut if that's the case, then #9 should work. His patch for poll.module makes it so only the nid and node title are cached, not $node->view. Moved back to core poll.module. Tell me if I'm thinking wrong.
Comment #22
chx CreditAttribution: chx commentedHold on for a second. Just because core does not utilize render caching at most places that does not mean contrib or site specific modules wont. If you are adding a PDO instance into the render array then Views kills render caching. Is that really a core bug? Childish? What about forum which IMO already uses render caching?
Comment #23
chx CreditAttribution: chx commented@Damien Tournoud please hold such judgements. I heard similar on IRC from you and it's not helping. We all want a better Drupal -- let us be constructive , OK?
In the name of constructive -- can't you just fetch all the results (if you dont already) andremove the query object from the view object? Am I talking nonsense here?
Edit: $node gets into #node in the render array, that's the problem.
Comment #24
Damien Tournoud CreditAttribution: Damien Tournoud commented@chx: I'm absolutely not judging. I just took what Earl wrote and rephrased it exactly on the opposite.
Childish? Definitely.
Comment #25
bfroehle CreditAttribution: bfroehle commentedWell, let's settle on a course of action. I think the following questions need to be answered:
$node
rather than just$node->nid
and$node->title
which it uses?$view
(which then gets attached to $node as #12 discusses)?I'm sending this back to Drupal Core to get additional eyes on it, but don't read this as casting my vote on the matter.
Comment #26
dmitrig01 CreditAttribution: dmitrig01 commentedThis is a core bug, working on fixing it.
Comment #27
dmitrig01 CreditAttribution: dmitrig01 commentedAnd the patch. Still need to write tests. Tomorrow.
Comment #28
dmitrig01 CreditAttribution: dmitrig01 commentedLots of thanks to chx btw
Comment #30
Blooniverse CreditAttribution: Blooniverse commentedTested on my system: Failed. Output below:
Below the two reject files as attachements:
Comment #31
Josh The Geek CreditAttribution: Josh The Geek commentedSo this is a database layer bug.
Comment #32
dmitrig01 CreditAttribution: dmitrig01 commentedWith test! And working!
Comment #33
dmitrig01 CreditAttribution: dmitrig01 commentedtiny comment fix
Comment #34
dmitrig01 CreditAttribution: dmitrig01 commentedand, very tiny fix
Comment #35
dmitrig01 CreditAttribution: dmitrig01 commentedoops
Comment #36
dmitrig01 CreditAttribution: dmitrig01 commentedbah
Comment #37
dmitrig01 CreditAttribution: dmitrig01 commentedonce and for all. maybe
Comment #38
chx CreditAttribution: chx commentedThat took a few tries :) setKey made me hm but then it's a copy of setTarget.
Comment #39
Crell CreditAttribution: Crell commentedSo for those playing our home game, here's what's going on here:
The cache system is trying to cache theme variables.
One of those theme variables is a $view object, exposed to the theme layer to give themers more stuff to play with.
The $view object references its own $views_query object.
If using a DB backend for Views, that $views_query object references a SelectQuery object, $select.
The $select object references the connection object to which it is bound.
The connection object is an instance of DatabaseConnection, which extends PDO.
PDO cannot be serialized, since it contains a resource.
So when you try to cache the theme variables, that works its way down the line to the connection object and explodes, which is exactly what the PHP engine says it should do.
The solution here uses PHP's serialization magic methods to break that last connection from $select to $conn when serializing and instead store the connection key and target, which as they're just strings serialize just fine. Then on wakeup, grab that connection singleton back based on those values. This is a great use of __sleep() and __wakeup(). :-)
Arguably we could wait to grab those keys until __sleep() instead of doing so in the constructor, but the performance difference should be undetectable. Technically, this does introduce new public methods on the connection object which may be considered a functionality change, but they're necessary to fix this bug and are, really, useless anywhere else so I consider them DB-layer internal. In hindsight having key and target be separate methods may or may not make sense but that's not something to play with at this point, especially since nothing outside the DB layer is playing with these anyway.
The only place this would break is if you serialize a connection object, change your database configuration in settings.php to have a different DB (or really, DB type) connected to a given key/target pair, and then unserialize the connection. If you're doing that, you're an idiot and there's no reason for us to support it.
So in short, +1 RTBC! :-) Thanks, Dmitri!
Comment #40
dmitrig01 CreditAttribution: dmitrig01 commentedYes, I asked chx about "we could wait to grab those keys until __sleep() instead of doing so in the constructor, but the performance difference should be undetectable" and he said it didn't matter either. And we don't support idiots.
Comment #41
Dries CreditAttribution: Dries commentedThanks for the stack trace in #39. That is very helpful. I don't know what the $view object is, and whether it is supposed to be extended in this way but let's assume that is OK.
I recommend adding a sentence explaining why the key representation is useful. It's not explained for setKey() and getKey() either.
Feel free to set back to RTBC after some documentation improvements. Thanks.
Comment #42
dmitrig01 CreditAttribution: dmitrig01 commentedThis is to Crell then, as I have no idea. This was simply copypasted from get/setTarget
Comment #43
dmitrig01 CreditAttribution: dmitrig01 commentedAfter talking in IRC:
$view object is the representation of a Views view, so in this instance what views is doing here makes perfect sense, and in fact, did this same thing in Drupal 6. The change here in Drupal 7 is the view object references a SelectQuery (which is the correct thing to do -- in Drupal 6 it was a query string), and in doing that we discovered that SelectQuery (well, all query objects) cannot be serialized.
The key is explained in database.inc circa line 302 (basically, key is a group of databases, and target is an individual database: in settings.php, $databases[$key][$target]), line 1513, and default.settings.php line 83. Putting this comment in ->get/setKey would basically just be repeating it again, hence my RTBC - it's already documented, just not right there.
Comment #44
webchickLine 302 in database.inc and friends is:
Line 83 and friends in settings.php is:
While the first one does mention the word "key" in that sentence somewhere, I wouldn't call that a definition. Furthermore if I was digging around in the code and wanted to look up what that variable was about, the place where it's defined is the logical place I would look, not some oddball other place.
Agreed with Dries on "needs work" for documentation. We don't need to go bananas, just define what the heck this key is in the place where its variable is declared.
Comment #45
dmitrig01 CreditAttribution: dmitrig01 commentedOk, I agree there. #32 was Crell+chx's thinking. Now with comments
Comment #46
dmitrig01 CreditAttribution: dmitrig01 commented#42 rather
Comment #47
chx CreditAttribution: chx commentedThere's docs :)
Comment #48
dmitrig01 CreditAttribution: dmitrig01 commentedperiods at end of sentences++
Comment #49
Crell CreditAttribution: Crell commentedI consider the additions in #48 unnecessary, but have no objection to them. Close enough for government work. Now let's get this committed.
Comment #50
carlos8f CreditAttribution: carlos8f commentedVery clever patch, nice work @dmitrig01!
Comment #51
carlos8f CreditAttribution: carlos8f commentedDoh.
Comment #52
dmitrig01 CreditAttribution: dmitrig01 commentedPutting docs in the right place
Comment #53
dmitrig01 CreditAttribution: dmitrig01 commentedbikeshedding comments in irc
Comment #54
crashtest_ CreditAttribution: crashtest_ commentedThe comments in this latest patch make it pretty clear to me what is going on, I give this a ++.
Comment #55
crashtest_ CreditAttribution: crashtest_ commentedOoooops!
Comment #56
webchickThose docs look good to me and have the Crell sign of approval.
Committed to HEAD. Thanks!
Dries if you want more details, please feel free to re-open.
Comment #57
dmitrig01 CreditAttribution: dmitrig01 commentedAwesome, thanks
Comment #58
Blooniverse CreditAttribution: Blooniverse commented... does anyone have a clue, why patching (#53) fails on my system?:patch < serialize_magic_2.patch[EDIT: add] Uh ... obviously wrong pathing. Sorry. Thanx, @int!
Comment #59
int CreditAttribution: int commentedpatch -p0 < serialize_magic_2.patch
Comment #60
Josh The Geek CreditAttribution: Josh The Geek commented@dmitrig01: It is magic this time! Thank you so much.