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 #492 (closed enhancement: fixed)

Opened 13 years ago

Last modified 12 years ago

[PATCH] Raise exception if a widget is not used in a threadsafe way

Reported by: kevin Owned by: anonymous
Priority: normal Milestone: 0.9
Component: TG Widgets Version:
Severity: normal Keywords:
Cc:

Description (last modified by kevin) (diff)

Widgets are designed to be reused for multiple requests. Those requests can happen simultaneously. That means that storing state as part of render() or insert() [via create_dict or otherwise] is not threadsafe and will likely cause people to see the wrong data.

An easy way around this is to set the __setattr__ 
method to something that doesn't allow changes 
at the end of __init__.

Attachments

threadedwidgets.diff Download (13.0 KB) - added by chadwickmorris@… 13 years ago.
Smallish patch and tests for raising exceptions after init

Change History

comment:1 Changed 13 years ago by kevin

  • Description modified (diff)

Changed 13 years ago by chadwickmorris@…

Smallish patch and tests for raising exceptions after init

comment:2 Changed 13 years ago by chadwickmorris@…

  • Summary changed from Raise exception if a widget is not used in a threadsafe way to [PATCH] Raise exception if a widget is not used in a threadsafe way

Patch doesn't do anything fancy, just setting self.instantiating=True, which allows setting attributes, then setting it to False at the end of the init method. Needed to move around certain things for subclasses that overloaded the init method, since the subclasses either need to call Widget's init after they're done setting their own attributes, or set self.instantiating themselves. That might need more thinking to make it easier to develop widgets. The datagrid widget fails its tests, since it modifies the self.dynfields attribute on update_dict, but everything else seems to check out.

comment:3 Changed 13 years ago by kevin

It was actually DataGrid that prompted me to open this ticket :)

Seeing that test makes me wonder if this particular approach is too heavyhanded. I guess it's worth trying it to see if people complain about not being able to set widget attributes after initialization.

A couple additional notes:

  • I'm in the midst of a major refactoring of widgets right now, so I can't apply this patch as-is. I could use the concepts though.
  • we could use the metaclass to decorate the init method with the initializing flag. that change would not require any special placement of a super.init. There may be a little trickery involved because the super method would also be decorated and would try to set self.initializing to False.

comment:4 Changed 13 years ago by kevin

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

My solution (committed in [619]) ended up using the metaclass to set a _locked flag when display() (the new form of insert()) is called. So, you can keep making changes to the object right up until the point that it's displayed. Once you start displaying the widget, it's locked.

Note: See TracTickets for help on using tickets.