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 20 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 20 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 20 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 20 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 20 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 20 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 19 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 .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from hasienda. Next status will be '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.