Issue with logout endpoint when using bakery module. [#1414344]

The logout functionality stops working after using services along with bakery module (http://drupal.org/project/bakery) and applying this patch (http://drupal.org/node/1369660#comment-5360870). In my opinion this actually is the way its supposed to work, calling the user_module_invoke('logout') hook before destroying the session, like core does -> http://drupalcode.org/project/drupal.git/blob/7c4e429b7fa771676a18321aac...

@@ -401,10 +401,10 @@ function _user_resource_logout() {
 
   watchdog('user', 'Session closed for %name.', array('%name' => theme('placeholder', $user->name)));
 
-  // Destroy the current session:
-  session_destroy();
   $null = NULL; // Only variables can be passed by reference workaround
   user_module_invoke('logout', $null, $user );
+  // Destroy the current session:
+  session_destroy();
 
   // Load the anonymous user
   $user = drupal_anonymous_user();

Comment	File	Size	Author
#5	1414344_sess_destroy_7.x-3.x_5.patch	844 bytes	cotto
#6	1414344_sess_destroy_7.x-3.x_5.patch	606 bytes	cotto
#1	move_logout_hooks_before_sess_destroy-1414344-1.patch	636 bytes	artkon

Comments

Comment #1

artkon commented 23 January 2012 at 17:01

Status	File	Size
new	move_logout_hooks_before_sess_destroy-1414344-1.patch	636 bytes

Comment #2

cotto commented 24 January 2012 at 00:07

Status:

Active

» Reviewed & tested by the community

After looking into this a bit, I agree with the #1 patch. Core Drupal 6.22 from d.o does not do what you said, but Pressflow does and the Pressflow approach makes sense to me after a bit of digging. Without the patch, it isn't possible for a hook_user logout implementation to mess with $SESSION data, which it should be able to do. It's not entirely surprising that this issue wouldn't be common since most modules don't care about implementing hook_user('logout', ...).

The patch makes sense and doesn't cause any new failures, so moving this to rtbc. Thanks for reporting!