Okay.. I've spent a considerable amount of time in the last 2 days debugging this and have finally figured out whats causing my parser_ical and Feeds configuration to completely fail (as in Core dumps).

Basically FeedsDateTime is extending DateTime, which is a php Built in object. Problem is now that feeds uses Batch, batch likes to serialize what its doing into the database. No go. You cant serialize a DateTime, and as such you cant serialize a FeedsDateTime either, because when it unserializes you will get something like this:

"The DateTime object has not been correctly initialized by its constructor." when doing standard methods like getTimezone(); etc.

http://bugs.php.net/39821

So I've added the following code to the FeedsDateTime class to get it to work for me. probably needs some test cases etc, but hey.. 

  private $_serialized_time;
  private $_serialized_timezone;
  
  public function __sleep(){
        $this->_serialized_time = $this->format('c');
        $this->_serialized_timezone = $this->getTimezone()->getName();
        return array('_serialized_time', '_serialized_timezone');
    }

    public function __wakeup() {
        $this->__construct($this->_serialized_time, new DateTimeZone($this->_serialized_timezone));
    }
CommentFileSizeAuthor
#5 parser_ical_batchfix.patch1.89 KBDavid Goode
#4 feeds-736684.patch1.37 KBSouvent22

Comments

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented

Should also add that when it unserializes the DateTime object, since it is no longer a properly constructed object, we run into this bug:

http://bugs.php.net/48476

Which is actually what causes the core dumps.

Souvent22’s picture

Souvent22 commented

Having a very similar issue; however, I am running both 5.2.3 and php 5.3. You'll notice in the bug reports you reference that there is a mention that serialization for the DT object was implemented in 5.3+, however I'm having the same issue in 5.2.3 and also when i switch over to 5.3. If anyone else is having this issue, could you guys also share your PHP version numbers you are running?

Souvent22’s picture

Souvent22 commented

P.S. Reference for 5.3 is:
http://bugs.php.net/41334 , however that person was not thorough enough to reference a patch/docs to verify, but is a core maintainer it seems.

Souvent22’s picture

Souvent22 commented
Version: 6.x-1.0-alpha12 » 6.x-1.x-dev
Assigned: Unassigned » Souvent22
StatusFileSize
new feeds-736684.patch1.37 KB

This is a diff off of HEAD/DEV. Spent a heck of a time tracking this bug down.

David Goode’s picture

David Goode commented
StatusFileSize
new parser_ical_batchfix.patch1.89 KB

Hey, thanks everyone, I'm looking into committing this. Could you give more specifics on how to reproduce this error? I'm importing ~50 nodes with 1 per batch from a standard RSS feed and mapped into a few different date objects and I don't get any such errors.

I do, however, get a different error from ical repeated dates, where the relevant date includes are assumed to be there but now aren't because they get included in different batches I suppose. Patch attached. Once both these are ironed out we should get this into cvs.

@edit ignore this patch, submitted in separate issue under parser_ical at #745370: Recurring iCal dates crash on batch import due to missing include.

Souvent22’s picture

Souvent22 commented

David,

Could you supply your PHP version number? And general ENV; I think this bug maybe PHP/Env specific/dependent.

David Goode’s picture

David Goode commented

Oh hey sry, I have PHP 5.3.1 on OSX, using Drupal 6.13 on this test, cvs head of feeds, date, parser_ical.

David

Souvent22’s picture

Souvent22 commented

Ah ok, that makes sense as, if what I'm reading from php.net, etc. is correct, this issue is resolved for 5.3+ ( 5.2.9+ really ), however for version before this, it will crash due to the fact that prior to 5.2.9, you can't serialize core php classes (e.g. DateTime).

So I think the solution, since the min PHP requirement for even Drupal 7 is only 5.2 that we will need this patch for those version before 5.2.8.

Others, can you guys post your PHP version numbers if you are having this problem so we can verify that this patch will solve the issue that is mentioned and that our use cause and thus solution hold?

Souvent22’s picture

Souvent22 commented

Side note:
When i use my 5.2.1 interpreter = crash, 5.3.0 interpreter = ok

alex_b’s picture

alex_b commented
Status: Active » Needs review

Souvent22: I'm not sure - the patch in #4 is RTBC from your point of view and only needs my review?

Souvent22’s picture

Souvent22 commented
Status: Needs review » Reviewed & tested by the community

alex_b: Yes, I believe so, afaik. I wanted to get more feed back though from peeps that had pre 5.2.1 php versions to see if the patch was working for them. But, AFAIK, RTBC with your review. Updating the status....though....I know as a general rule, one is not supposed to change one's own patch to RTBC; however technically it's a re-rolling of the original posters fix/patch.

alex_b’s picture

alex_b commented
Status: Reviewed & tested by the community » Fixed

Ok, this is committed. Thank you everybody.

David Goode: the patch in #5 is covered with #745370: Recurring iCal dates crash on batch import due to missing include, correct?

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.