Modify

#10625 closed defect (fixed)

AssertionError in trac.db.pool.PooledConnection.__del__

Reported by: anonymous Owned by: hasienda
Priority: normal Component: AccountManagerPlugin
Severity: normal Keywords: database API
Cc: Trac Release: 1.0

Description (last modified by hasienda)

On login and logout I get the following error (for each environment):

Exception exceptions.AssertionError: <exceptions.AssertionError instance at 0x89faeacc> in <bound method PooledConnection.__del__ of <trac.db.pool.PooledConnection object at 0xb47cc88c>> ignored

It is related with the single sign on feature. It can be easily fixed by explicitly closing the database connection with db.close after the commit in the following lines:

Attachments (0)

Change History (13)

comment:1 Changed 21 months ago by ms4py

Well, a cleaner fix would be to upgrade the database access in general.

comment:2 Changed 21 months ago by ms4py

  • Cc ms4py added

comment:3 follow-up: Changed 21 months ago by ms4py

I have upgraded the plugin to use the Trac 1.0 database API. Let me know if you are interested in the patch.

comment:4 in reply to: ↑ description Changed 21 months ago by hasienda

  • Description modified (diff)
  • Keywords database API added

Replying to anonymous:

On login and logout I get the following error (for each environment):

Exception exceptions.AssertionError: ...

It is related with the single sign on feature. It can be easily fixed by explicitly closing the database connection with db.close after the commit in the following lines:

I strongly disagree, and this is backed by developer discussion. A forced close may yield other side-effects, and current Trac db API strongly advocates for not using such forceful actions to a transaction in general.

comment:5 in reply to: ↑ 3 ; follow-up: Changed 21 months ago by hasienda

Replying to ms4py:

I have upgraded the plugin to use the Trac 1.0 database API. Let me know if you are interested in the patch.

I already suspected from last comment, that you meant specifically the Trac-1.0 db API.

Sure, this will happen, but keep in mind, that AccountManager is in rather widespread use. Maintaining backwards-compatibility, at least down to Trac-0.11, is a major development constraint for me. How could I do that with the new API? I know, that its adoption will yield significant improvements, but I'm sure I can't drop support for Trac-0.11 to 0.12.x anytime soon.

My plan is to fork 1.0 development later on, so your patch would give a jump-start and therefore is certainly welcome. But now it's much to early for that, especially 'no-way' before the upcoming acct_mgr-0.4 release, that I've prepared for months now.

comment:6 Changed 21 months ago by hasienda

I've been testing the SSO feature for stability over about 10 months in production. I had my Trac processes crashing on me even more nastily than reported here for month. But lately I got a grip even on the most random issues. I'll update the code accordingly, probably just not in a way that you expect by now.

comment:7 in reply to: ↑ 5 Changed 21 months ago by ms4py

Yes, it is obvious that an 1.0 targeting patch will require a new branch :) I will post the patch next week (don't have access to my Trac instance from here).

About the issue in general: I'm not quite sure if it is really the plugin which is causing the problem. It seems that it is implemented strictly according to the API. I think there is a design flaw in the Trac database API itself because it relies somehow on the garbage collector to close connections. This is a very bad style which is probably causing this issue as a side effect.

Please keep me posted here about your changes you mentioned above. I'm curious if they solve this issue, too.

comment:8 Changed 21 months ago by hasienda

(In [12378]) AccountManagerPlugin: Prevent SSO functionality from stumbling on db issues, refs #9676 and #10625.

I've been exploring seemingly random errors for a couple of months, starting
with the introduction of single-sign-on support code in r11127.
It turned out to be caused by (SQLite) db access issues concealed behind more
than a dozen unique trace-backs, many of them even not yielding a hint on
their connection to attempted/prior Trac db access at all.

Sure, the forced environment shutdown was a first good candidate to sort them
out, but only turning on Trac environment caching made Trac finally running
stable with the new SSO code in place.

As a follow-up on changeset [11131] I've futher reduced the session cookie
update frequency after taxing the probability of a session cookie exploit at
approximately 1:2*1038 per forged HTTP request, to reduce db burden by this
feature as well.

comment:9 Changed 21 months ago by ms4py

  • Cc ms4py removed

Just did a short test with [12378] and everything seems fine. I'll do a longer test in production and will notify you if anything occurs again.

In this case I think we don't need my patch at all, it was basically just a replacement of db = env.get_db_cnx() by with self.env.db_transaction as db:.

comment:10 Changed 21 months ago by hasienda

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

(In [12398]) AccountManagerPlugin: Releasing version 0.4, pushing development to acct_mgr-0.5dev.

Availability of that code as stable release
closes #874, #3459, #4677, #5295, #5691, #6616, #7577, #8076, #8685, #8770, #8791, #8990, #9052, #9079, #9090, #9139, #9246, #9252, #9547, #9618, #9676, #9843, #9852, #9940, #10023, #10028, #10123, #10142, #10204, #10276, #10397, #10412, #10594, #10625 and #10644.

Some more issues have been worked-on, yet without confirmed resolution,
refs #5464 (for JiraToTracIntegration), #8927 and #10134.

And finally there are some issues and enhancement requests showing progress,
but known to require more work to resolve them satisfactorily,
refs #843, #1600, #5964, #8217, #8933.

Thanks to all contributors and followers, that enabled and encouraged a good
portion of this development work.

comment:11 Changed 21 months ago by ms4py

  • Resolution fixed deleted
  • Status changed from closed to reopened

There is another shutdown causing the same issue (in r12378 and in r12398). Not sure why I missed this before. Removing this line should finally fix it:

http://trac-hacks.org/browser/accountmanagerplugin/0.11/acct_mgr/web_ui.py#L614

comment:12 Changed 21 months ago by hasienda

(In [12429]) AccountManagerPlugin: Finally fix db issues for SSO functionality, refs #9676 and #10625.

This is an urgent follow-up to [12378], altering another critical occurrence
of cross-environment Trac db access, that I've been overlooking before.

Thanks to ms4py for testing and hinting on this remaining issue.

comment:13 Changed 20 months ago by hasienda

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

(In [12482]) AccountManagerPlugin: Publish maintenance release 0.4.1, closes #5964, #8545, #10134, #10625, #10700 and #10701.

This is an update for current stable acct_mgr-0.4 with a number of fixes for
issues resolved within the last weeks, i.e.:

  • a final fix for Single-Sign-On functionality (refs #9676),
  • a long-standing HttpAuth login issue and
  • one for acct_mgr.LoginModule, that is relevant if used with web-servers, that evaluate the REMOTE_USER environment variable.

Changeset [12468] is included, that may require a Trac db fix-up.
Run python ./contrib/fix-session_attribute-failed_logins.py <env> once on any
Trac environment, that had account locking enabled with time constraints
before.

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.