Session handler executed after $user object is destructed

chx - November 10, 2006 - 00:11
Project:Drupal
Version:4.6.x-dev
Component:base system
Category:bug report
Priority:normal
Assigned:chx
Status:closed
Description

The PHP docs clearly mention this since 5.0.5. Why Drupal was wrorking with previous versions? Mystery. But 5.2 seems to have brought this up.

There was another issue out of this but that was too littered.

AttachmentSize
sess_patch707 bytes

#1

chx - November 10, 2006 - 00:15

Before I forget. Can this break something? Unlikely. If you have an object that changes $user in the destructor or save something into SESSION then that will break, but objects especially objects with destructors (ie. non stdClass objects) is alien to Drupal world. This matter because we will need to port this back to every Drupal we care of.

#2

chx - November 10, 2006 - 00:17

Some sneaky chars in my patch which are invisible to my editor :(

AttachmentSize
sess_patch_0702 bytes

#3

AjK - November 10, 2006 - 00:22

subscribe to issue

#4

chx - November 10, 2006 - 00:38

Forgot to say thanks for ajk who did a lot of gruntwork and uptimebox who lended access to a broken box.

#5

AjK - November 10, 2006 - 00:40

OK, I went through the old issue and where people stated that certain things weren't fixed by this patch (notably cron and update.php) I tested this patch and it worked ok. So that's good.

For those that said "it didn't work in other places", sorry I could not test that, not enough information! So, if you are going to reply to this issue please be verbose and give us pages that don't work. One liners that say "didn't work" aren't helpful to debugging / testing.

#6

NaX - November 10, 2006 - 07:19

Have you guys looked at this http://www.php.net/manual/en/function.session-write-close.php#54838 for the file_transfer($source, $headers) function. When a user is downloading large files the session is locked and all other pages wait for the page to finish downloading.

I think we need to call session_write_close(); before the file start downloading.

#7

chx - November 10, 2006 - 10:09

Nax, that's a whole different issue.

#8

fago - November 10, 2006 - 13:53

thanks chx for your patch. I've tried it and it seems to work fine.

I had tested the same line, but I had this line in the index.php after the drupal_bootstrap(). Presumable this was too late, so I had problems with this.

I'm using this now and I'll report if any troubles occur again.

#9

satori@abc3400.be - November 10, 2006 - 14:32

+1

Just subscribing.

#10

chx - November 10, 2006 - 15:02
Status:patch (code needs review)» patch (reviewed & tested by the community)

If fago, the original reporter also says it's OK (and ajk did a very through testing on an also known-to-broken box) then we are good to go.

#11

laura s - November 10, 2006 - 15:36

Assuming this will not work for 4.7.... subscribing.

#12

AjK - November 10, 2006 - 15:41

Once this goes into HEAD we'll backport the fix to 4.7 (and maybe 4.6)

#13

satori@abc3400.be - November 10, 2006 - 16:37

laura s: I've manually merged this patch with my 4.7 session.inc and everything seems to be in order.

#14

jhm - November 10, 2006 - 17:29

I am one of those who CAN reproduce the behavior every-time under

Mac OS X 10.4.8
Apache1.3
PHP5.2.0 (www.entropy.ch Release 3)
MySQL 5.0.24a-standard

but only when using the mysql driver. The problem goes away as soon as I specify the mysqli driver.

Therefore I am dubious that the proposed "patch" to register a shutdown function in sess_read is the right approach.

My 2cents.

#15

drewish - November 10, 2006 - 17:53

this patch fixed it for me.

#16

fago - November 10, 2006 - 19:33

by using mysqli the issue doesn't occur, because of the mysqli db layer of drupal, which does already address this problem the same way

#17

fax8 - November 10, 2006 - 20:26

I was having that problem. this patch fixed it.

Fabio Varesano

#18

Arto - November 10, 2006 - 20:35

Attached is Karoly's patch rolled against DRUPAL-4-7. Fixed the issue with Drupal 4.7.4 running on PHP 5.2.0 + Apache 2.2.3 + MySQL 4.1.20.

AttachmentSize
sess_patch_47632 bytes

#19

drewish - November 10, 2006 - 20:35

a bit more feedback: i applied the fix by hand to two 4.7 sites, one running php 5.1, the other php 5.2. both are working fine now.

#20

jhm - November 10, 2006 - 20:58

by using mysqli the issue doesn't occur, because of the mysqli db layer of drupal, which does already address this problem the same way

indeed, adding the snippet from database.mysqli.inc:db_connect to database.mysql.inc:db_connect solves the issue.

So my question is, why adding the same patch to a different file (session.inc)? Doesn't that mean that session_write_close is getting called twice if I apply the patch AND use mysqli?

#21

chx - November 11, 2006 - 18:52
Version:x.y.z» 5.0-beta1

#22

drumm - November 11, 2006 - 22:41
Status:patch (reviewed & tested by the community)» fixed

Committed to HEAD.

#23

Arto - November 12, 2006 - 00:33
Version:5.0-beta1» 4.7.4
Status:fixed» patch (reviewed & tested by the community)

Please also apply the patch in #18 to the DRUPAL-4-7 branch.

#24

FiReaNG3L - November 12, 2006 - 04:58

Just encountered this problem by upgrading to latest WAMP on test box (contains PHP 5.2.0) in drupal 4.7. The patch in #18 fixed the problem.

#25

killes@www.drop.org - November 12, 2006 - 17:15
Status:patch (reviewed & tested by the community)» fixed

applied

#26

Veggieryan - November 13, 2006 - 22:19

this works for me as well on latest WAMP5 on windows vista

#27

jhm - November 13, 2006 - 23:33

after discussion this on #drupal-support, I created a new patch file that adds register_shutdown_function to session.inc:sess_read and removes same call from database.mysqli.inc:db_connect.

This patch file is for drupal4.7.4. It shouldn't be hard to create one for drupal5.beta-1

AttachmentSize
session_2.patch1.55 KB

#28

chx - November 13, 2006 - 23:37
Version:4.7.4» 5.x-dev
Priority:critical» normal
Status:fixed» patch (reviewed & tested by the community)

No need to two pieces indeed... your patch moved it to sess_read for 4.7 but that's already committed. Please apply this patch to 4.7 and HEAD too.

AttachmentSize
remove_sess_write_close.patch782 bytes

#29

Dries - November 15, 2006 - 19:48
Version:5.x-dev» 4.7.x-dev

Committed to CVS HEAD.

#30

arthurf - November 16, 2006 - 03:48

Just wanted say thanks for this patch, it saved my hide when my host updated!

#31

Alulim - November 16, 2006 - 21:17

There is still a problem when trying to attach a file. When a file is selected and the submit button is pressed, I get kicked out of the system. The problem with PHP 5.2.0 is not completely solved.
Anybody looking at this?
Or is this issue handled in another node as this problem is likely to be in the upload module?

#32

ceardach - November 17, 2006 - 17:33

I applied only patch #18 and was able to remain logged in and upload files.

My system is:
Apache/1.3.37 (Unix)
PHP/5.2.0

#33

Alulim - November 19, 2006 - 12:50

OK, my file uploading problem is solved too.
The problem was the combination of 2 patches. I removed the very first patch which was issued for this problem.
Currently only using patch sess_patch_47 and everything is working fine again.

#34

thepeter - November 24, 2006 - 20:21

Hi someone told me that this patch could help me with problem decsribed here (imce upload files problem) I have applyed this patch but nothing has changed and by checking the uploading attachments for node also doesn't work. drupal is 4.7.3 php is 5.2 apache 1.actual (don't know precise version) and Iam using mysqli extension

#35

webchick - November 24, 2006 - 22:10
Status:patch (reviewed & tested by the community)» patch (to be ported)

I think this is the right status.

#36

inforeto - November 27, 2006 - 22:39

Subscribing.

#37

killes@www.drop.org - November 30, 2006 - 20:17
Status:patch (to be ported)» fixed

if there is no work neccessary RTBC is IMO the right status. applied

#38

killes@www.drop.org - November 30, 2006 - 20:17

if there is no work neccessary RTBC is IMO the right status. applied

#39

magico - December 4, 2006 - 16:25
Version:4.7.x-dev» 4.6.10
Status:fixed» patch (to be ported)

Subscribing, so it can be ported to 4.6.x

#40

magico - December 4, 2006 - 17:49
Status:patch (to be ported)» patch (code needs review)

Here it is. I've tested and it seems right.
BTW: can someone create a 4.6.x-dev please? Thanks

AttachmentSize
session.inc.patch_5.txt646 bytes

#41

magico - December 5, 2006 - 15:41
Status:patch (code needs review)» patch (reviewed & tested by the community)

I received the following message:

I described my recent problem in http://drupal.org/node/101424
I was guided to your patch and was succesfull in installing. Now I can
stay logged and work with Drupal 4.6.6 site.

Php 5 has been installed about a year ago, Drupal 4.6.6 half a year ago,
server last booted more than a month ago. Nobody didn't do nuthin' and
suddenly this problem occurs.

With best thanks Timo Sylvänne

Marking this RTBC.

#42

magico - December 15, 2006 - 20:19
Version:4.6.10» 4.6.x-dev

#43

Mguel - December 22, 2006 - 23:28

As mentioned at:
Troubleshooting FAQ > Login problems on PHP 5.2

I applied patch at:
http://drupal.org/node/93945#comment-155161
which is comment #27 of this thread.

After applying the patch the problem persists when using Firefox 2.0. With konqueror I don't have the problem, but I'm not sure if it is because of the patch, or it worked before (I didn't test it with konqueror before applying the patch).

Thanks,
Mguel

PS: shoudn't apache be restarted after applying the patch?

#44

relyod - December 22, 2006 - 23:43
Version:4.6.x-dev» 4.7.4

I manually applied this patch to my system (freshly installed 4.7.4) For less php-aware users, can we roll a 4.7.4 version of this
patch, since the patch didn't want to apply since it was from the 5.0beta branch ?

Also, to the poster who asked "why previous versions worked", the answer is probably that they were using an earlier version of PHP, since I first saw the problem after updating PHP, rather than after updating Drupal! - Luckily I updated PHP on my test system first, rather than on a production system, of which I have several

#45

killes@www.drop.org - January 1, 2007 - 18:04
Version:4.7.4» 4.6.x-dev

this patch is already committed for 4.7 and will appear in 4.7.5.

#46

magico - January 2, 2007 - 11:02

Can someone look at #40? It has a small patch needing some commit...

#47

killes@www.drop.org - January 4, 2007 - 21:29
Status:patch (reviewed & tested by the community)» fixed

appleid to 4.6

#48

Chill35 - January 7, 2007 - 22:01

What behavior is this supposed to fix ?

#49

Anonymous - January 21, 2007 - 22:15
Status:fixed» closed

#50

ChrisKennedy - January 22, 2007 - 20:17

I upgraded a site running on PHP 4.3.11 from 4.7 to 5.0 only to find myself completely unable to login. I checked and no rows were being stored in the sessions db table. This was pretty disturbing and after experimentation with numerous cookie/domain setting combinations, I have discovered that commenting out this change fixed the problem.

This might also be causing the issue at http://drupal.org/node/108663#comment-182479

#51

kweisblatt - February 26, 2007 - 19:53
Title:Session handler executed after $user object is destructed» applying this patch

I have no idea how to apply this. I have applied patches using cygwin before with specific files, but this is over my head.

Any help is appreciated.

#52

asimmonds - February 26, 2007 - 21:08
Title:applying this patch» Session handler executed after $user object is destructed

Reverting title

#53

Clemens - February 20, 2008 - 22:20

Manually applied patch in #40 to 4.6.5 session.inc for a perfect fix...
Thank you Magico

 
 

Drupal is a registered trademark of Dries Buytaert.