Cannot serialize or unserialize PDO instances error

I 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.

Maybe 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:

#0 DrupalDatabaseCache->set
#1 cache_set
#2 form_set_cache
#3 drupal_process_form
#4 drupal_build_form
#5 drupal_get_form
#6 poll_view
#7 node_invoke
#8 node_build_content
#9 node_view
#10 node_view_multiple
#11 template_preprocess_views_view_row_node
#12 theme
#13 views_plugin_row->render
#14 views_plugin_style->render
#15 template_preprocess_views_view
#16 theme
#17 views_plugin_display->render
#18 view->render
#19 views_plugin_display_block->execute
#20 view->execute_display
#21 views_block_view
#23 module_invoke
#24 _block_render_blocks
#25 block_list
#26 block_get_blocks_by_region
#27 block_page_build
#28 drupal_render_page
#29 drupal_deliver_html_page
#30 drupal_deliver_page
#31 menu_execute_active_handler

Maybe it's because we're caching the 'poll_view_voting' form which contains the following fields:

   // Store the node so we can get to it in submit functions.
  $form['#node'] = $node;
  $form['#block'] = $block;

and Views is adding some sort of database objects to $node?

I 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.

This 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.

The 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:

  $node->view = $vars['view'];

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.

subscribing,

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,

Setting to needs work, since that's what this issue needs.

... 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).

Most likely what's going on is that the view object is being added to the node during the render process.

It is exactly it. That means it's a Views bug: Views makes the $node object not serializable.

Or perhaps it is a problem with the poll module --- do we really need to serialize ALL of $node?

Fine 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.

There 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.

Hold 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?

@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.

@chx: I'm absolutely not judging. I just took what Earl wrote and rephrased it exactly on the opposite.

Childish? Definitely.

Well, let's settle on a course of action. I think the following questions need to be answered:

1. Is it important that poll.module cache all of $node rather than just $node->nid and $node->title which it uses?
2. Is it important that Views puts the PDO objects in $view (which then gets attached to $node as #12 discusses)?
3. Is there a way to make PDO 'serializable' in the sense that it doesn't throw an error (but obviously wouldn't be functional either)?

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.

This is a core bug, working on fixing it.

And the patch. Still need to write tests. Tomorrow.

Lots of thanks to chx btw

Tested on my system: Failed. Output below:

root@ubuntu:/var/www/website.net# patch < database.inc.patch
patching file database.inc
Hunk #1 FAILED at 194.
Hunk #2 FAILED at 470.
Hunk #3 FAILED at 1539.
3 out of 3 hunks FAILED -- saving rejects to file database.inc.rej
patching file query.inc
Hunk #1 FAILED at 242.
Hunk #2 FAILED at 270.
2 out of 2 hunks FAILED -- saving rejects to file query.inc.rej

Below the two reject files as attachements:

With test! And working!

tiny comment fix

and, very tiny fix

oops

bah

once and for all. maybe

That took a few tries :) setKey made me hm but then it's a copy of setTarget.

So 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!

Yes, 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.

Thanks 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.

+++ includes/database/database.inc	11 Dec 2010 20:43:39 -0000
@@ -194,6 +194,13 @@ abstract class DatabaseConnection extend
+   * The key representing this connection.

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.

This is to Crell then, as I have no idea. This was simply copypasted from get/setTarget

After 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.

Line 302 in database.inc and friends is:

   * - target: The database "target" against which to execute a query. Valid
   *   values are "default" or "slave". The system will first try to open a
   *   connection to a database specified with the user-supplied key. If one
   *   is not available, it will silently fall back to the "default" target.
   *   If multiple databases connections are specified with the same target,
   *   one will be selected at random for the duration of the request.

Line 83 and friends in settings.php is:

 * For each database, you may optionally specify multiple "target" databases.
 * A target database allows Drupal to try to send certain queries to a
 * different database if it can but fall back to the default connection if not.
 * That is useful for master/slave replication, as Drupal may try to connect
 * to a slave server when appropriate and if one is not available will simply
 * fall back to the single master server.

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.

Ok, I agree there. #32 was Crell+chx's thinking. Now with comments

#42 rather

There's docs :)

periods at end of sentences++

I consider the additions in #48 unnecessary, but have no objection to them. Close enough for government work. Now let's get this committed.

Very clever patch, nice work @dmitrig01!

Doh.

Putting docs in the right place

bikeshedding comments in irc

The comments in this latest patch make it pretty clear to me what is going on, I give this a ++.

Ooooops!

Those 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.

Awesome, thanks

... 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!

patch -p0 < serialize_magic_2.patch