Fix some services that shouldn't be serialized

Without this patch using php > 5.3 Drupal\user\Tests\UserCancelTest fails. With it was get a pass. This could also ease memory issues whilst testing on php 5.3.

Something else to consider for DependencySerialization would be to specifically check for the container too, for ContainerAware things that get serialized. Not something that's actual problem right now, but worth considering.

The urlGenerator issue is huge actually, because that contains the container due to having th event stuff injected, so we dump all that. And shows how easy it is to mess things up.

Also noticed that #2042773: Change #items within a theme_field() render array from an *array* to the same $items *object* used throughout the rest of the formatter pipeline means that we serialize the field item list objects including their definitions if the resulting render array gets cached, which happens for example for preview.

I'm ok with this being committed and doing looking at further optimizations, though.

I wrote a short script that dumps a (large) serialized string in a readable way, works pretty well, in case someone needs it for debugging this stuff: https://gist.github.com/Berdir/8848201

+1 to RTBC, that was a crosspost :)

To give you an idea of the impact of the urlGenerator change, preview a comment, then check key_value_expire:

HEAD:
form: ~490kb
form_state: ~486kb

With patch:
form: ~20kb
form_state: ~15kb

Okay give #5 and the earlier HEAD fails for:

Fatal error: Allowed memory size of 268435456 bytes exhausted (tried to allocate 536870912 bytes) in /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Connection.php on line 336

Committed 5bcaf27 and pushed to 8.x. Thanks!

Nice work amateescu!

Those hidden serialization bloats are worryingly easy to introduce, and worryingly difficult to notice - and that's core alone, it scares me to think of what it will become in contrib :-/

Wondering wheher it would be doable to move away from PHP's serialize() and use a custom drupal_serialize() alternative that would let us at least watchdog() when a service (something with a ->_serviceId property) gets serialized...

Yes, very worried about that too.

I'm not sure if that could work, always checking that might result in quite some overhead.

What about putting a __sleep() { throw new Exception('Don't do that!') } in the Container class? That's really the worst one, everything else is peanuts compared to serializing the container.

As a start, might be useful to let testbot run with that to see if we're doing it anywhere?

Opened an issue for that: #2190643: Serializing the container is a very very bad idea, let's prevent it?

@Berdir #3

we serialize the field item list objects including their definitions if the resulting render array gets cached, which happens for example for preview

Hmm. Preview is handled very differently on the various entity types that support it :-/

Opened #2190767: Preview shouldn't mean serializing the render arrays