Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #397 (closed defect: fixed)

Opened 13 years ago

Last modified 12 years ago

TG_visit initialization threading problem

Reported by: alberto@… Owned by: anonymous
Priority: high Milestone:
Component: SQLObject Version:
Severity: normal Keywords: threading sqlobject visit
Cc: randall@…, ianb@…

Description

A I've mentioned in the  group, there is a problem with SQLObject's object initialization when various threads try to init at the same time the same class. It probably affects all sql objects so some fix should go upstream to SQLObject. Anyway, it's particularily problematic with TG_Visit as every request (potentially) creates a new TG_Visit object.

The attached patch works around it and applies Egor Cheshkov's  suggestion of making the visit id more random, therefore more secure.

Attachments

threading.patch Download (1.6 KB) - added by alberto@… 13 years ago.
the patch
sqlobject_threading.patch Download (1.4 KB) - added by alberto@… 13 years ago.
pathc for sqlobject's declarative.py
visit.patch Download (1.0 KB) - added by alberto@… 13 years ago.
patches visit.py
visit.2.patch Download (1.1 KB) - added by alberto@… 13 years ago.
yikes, wrong (obsolete) patch uploaded before
visit.3.patch Download (1.1 KB) - added by alberto@… 13 years ago.
I'm feeling stupid today. Previous visit.patch had a typo, sorry
test_SO_init.py Download (3.0 KB) - added by alberto@… 13 years ago.
This script reporoduces the threading problem. Run it a couple of times as most of the time doesn't raise anything (damn threads ;)

Change History

Changed 13 years ago by alberto@…

the patch

comment:1 Changed 13 years ago by alberto@…

I'm attaching two patches that make my previous one obsolete:

One fixes the root of the problem, it patches sqlobject's declarative.py so EVERY sqlobject created has it's init method wrapped in acquire/release. Tested and works wonderfully for me. I'll post at sqlobject's Trac when I find it (¿where is it?, I'm not feeling lucky® today...)

The other one patches visit.py so key generation is more secure.

Changed 13 years ago by alberto@…

pathc for sqlobject's declarative.py

Changed 13 years ago by alberto@…

patches visit.py

Changed 13 years ago by alberto@…

yikes, wrong (obsolete) patch uploaded before

Changed 13 years ago by alberto@…

I'm feeling stupid today. Previous visit.patch had a typo, sorry

comment:2 Changed 13 years ago by alberto@…

I've just filed sqlobjects patch at its tracker at sourceforge

comment:3 Changed 13 years ago by alberto@…

I've separated the visit patch on a separate ticket #407 as they address totally different problems. Please ignore visit*patch

comment:4 Changed 13 years ago by kevin

Some fix for this has to be in 0.9, but I'd like to see if we can get the root cause fix applied to sqlobject. If not, we'll apply the fix to our class.

comment:5 Changed 13 years ago by Jeff Watkins

  • Status changed from new to closed
  • Resolution set to fixed

Instead of kludging locks into the constructor, I create the visit record inside a transaction. The hub.end() call clears the cache, so this object should never be retrieved by any other call. It's a shame that SQLObject's caching and constructors are broken, but there's not telling when they'll be fixed.

Fixed in r548.

comment:6 Changed 13 years ago by ianb@…

  • Status changed from closed to reopened
  • Severity changed from critical to normal
  • Cc randall@…, ianb@… added
  • Component changed from Identity to SQLObject
  • Milestone 0.9 deleted
  • Resolution fixed deleted

I'm not really clear on what the problem is. The lock on __init__ seems kind of extreme, and I wouldn't want to apply that (especially not in declarative, because __init__ is not necessary unthreadsafe).

Randall -- it is mentioned that you had the same problem as described in this ticket. Can you look at this and perhaps expand on the issue? I'm having a hard time seeing how _SO_finishCreate can get called twice. Only _SO_finishCreate deletes the _creating attribute. Only _create() calls _SO_finishCreate. Only __init__ calls _create. And __init__ can't be entered twice on the same instance, because no one else should have access to the object until after that function returns.

If someone has a reproduceable example I might be able to track down the real issue.

comment:7 Changed 13 years ago by kevin

I think I see the problem.

_init is called by _SO_finishCreate, but it's _init that sets self.sqlmeta! So, when self.sqlmeta._creating is set, it's being set on the sqlmeta class, not an instance of sqlmeta. At least, that's my theory.

That would mean that the likely fix would be to do self.sqlmeta = self.class.sqlmeta(self) in init or somewhere else that comes before _create.

comment:8 Changed 13 years ago by kevin

  • Summary changed from [PATCH] TG_visit initialization threading problem to TG_visit initialization threading problem

comment:9 Changed 13 years ago by alberto@…

I'll try to write a test-case somewhen today...

Changed 13 years ago by alberto@…

This script reporoduces the threading problem. Run it a couple of times as most of the time doesn't raise anything (damn threads ;)

comment:10 Changed 13 years ago by alberto

  • Status changed from reopened to closed
  • Resolution set to fixed

sqlobject_threading.patch was applied to sqlobject at rev. 1547

Note: See TracTickets for help on using tickets.