Thread safety

[robcharlwood]

Hello,

This package does not currently appear to be thread safe. When running this package under gunicorn with multiple workers (I used 4 for my sample), flex select fails with the below traceback. For your reference my setup was django 1.5.5 on Python 2.7.3 with nginx proxying to gunicorn 18.0.

2013-11-18 08:57:51,967 django.request ERROR Internal Server Error: /flexselect/field_changed
Traceback (most recent call last):
  File "/home/xxx/xxx/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 115, in get_response
    response = callback(request, *callback_args, **callback_kwargs)
  File "/home/xxx/xxx/local/lib/python2.7/site-packages/django/contrib/auth/decorators.py", line 25, in _wrapped_view
    return view_func(request, *args, **kwargs)
  File "/home/xxx/xxx/local/lib/python2.7/site-packages/flexselect/views.py", line 17, in field_changed
    widget = FlexSelectWidget.instances[hashed_name]
KeyError: u'_7f7204089cbe1187f19ebc6d47c87c273bb9bb52'

I think this is due to the way that the FlexWidgets are being stored in the module/memory. I am currently working to find a fix to the issue. I will raise a pull request when I have something that works.

Many thanks,
Rob

[runekaagaard]

Hi Rob

I have it working on two similar setups but way older django versions. But I agree, the FlexSelectWidget.instances[self.hashed_name] = self line in the init function of the FlexSelectWidget class looks suspicious and would be the plade i'd start looking.

If the solution isn't obvious, let me know and I'll have a look too. And thx for improving this proejct!

Cheers
Rune

[robcharlwood]

Your welcome Rune. :)

I will take another proper look at this tonight and see if I can find a neat and tidy, backwards compatible solution. :)

Many thanks,
Rob

[runekaagaard]

Hmmm - Am getting similar errors. I think a register approach would be better, no? It's kind of unicorny.

[robcharlwood]

@runekaagaard

Hi Rune,
I am actually half way through a fix for this but haven't had any time over the last few months to finish it off.

I shall try and take some time this weekend to get a pull request setup for you.

Many thanks,
Rob

[runekaagaard]

I know about not having time. I could use 72 hours a day easily :) If you do have time that would be awesome! I've been relearning why I did the hack, and it's actually pretty clever. But I reckon the correct solution is to get the form the same way that modeladmin does it? Or what's your approach?

[robcharlwood]

Hi Rune,

A 72 hour day sounds good! :) I think we should start a pertition!

No worries, I will try and get some time at the weekend and raise a pull request. My solution utilizes a registration based approach which should be considerably less error prone when running in a multi-thread environment. I would need to look to see what the django model admin does to return the form but a neater solution may possibly reveal itself by checking what django does first. I shall be sure to check it out before I raise a request with my registration based approach.

Cheers,
Rob

[Karmak23]

Ping @runekaagaard @robcharlwood

The fix could be easily done using Redis to store the hashes.

As I will need it in the coming weeks in a production project, I will implement it.
But it will probably be a 3-lines rush patch needing to be properly documented.
BTW, I think requiring every user to install redis could seem an overkill, but it easily solves any multi-{thread,process,machine} problem :-D

regards,

[twerp]

Ping @Karmak23 @robcharlwood

1. I've been trying to update this app and the included example project for Django 1.8 and Python 3, and I did bump into this KeyError thing, too. Now I seem unable to reproduce it, though.

2. I do wonder why FlexSelectWidget's __init__() gets called three times during page load (Add/change case in the example). Could it have some influence on this issue?

3. My main problem right now is that changing to the "empty" option at any time results in a RelatedObjectDoesNotExist error. That one happens also when you add a new case and select any client, which may be harder (for me) to fix. Any help is appreciated :)

[runekaagaard]

I've just added a fix, if you want to test it, go right ahead :)

[runekaagaard]

Check the commit message for how to do it: 2d6325b