user warning: Duplicate entry '1' for key 1 query: INSERT INTO session_restore VALUES (1, 'a:6:{s:8:\"textsize\";s:2:\"70\";s:17:\"updates_remaining\";a:0:{}s:20:\"node_overview_filter\";a:0:{}s:18:\"ad_overview_filter\";a:0:{}s:21:\"dblog_overview_filter\";a:0:{}s:24:\"watchdog_live_latest_wid\";s:7:\"8363545\";}') in /var/www/web3/html/sites/all/modules/session_restore/session_restore.module on line 83.

CommentFileSizeAuthor
#4 session_restore_race_condition-517884-4.patch2.41 KBiva2k

Comments

Zen’s picture

Zen commented
Status: Active » Postponed (maintainer needs more info)

I am unable to replicate this. Please provide more information.

Zen’s picture

Zen commented
Status: Postponed (maintainer needs more info) » Closed (cannot reproduce)
iva2k’s picture

iva2k commented
Version: 6.x-1.1-beta » 7.x-1.0-alpha1
Status: Closed (cannot reproduce) » Active

It occasionally happens in 7.x:

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '1' for key 'PRIMARY': INSERT INTO {session_restore} (uid, session) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1); Array ( [:db_insert_placeholder_0] => 1 [:db_insert_placeholder_1] => a:7:{...} ) in _session_restore_session_set() (line 91 of .../sites/all/modules/session_restore/session_restore.module).

It is hard to reproduce (had something to do with having open session logged in as admin, closing the browser, then reloading the browser and letting it restore the old session which opened a bunch of tabs on the website at once -- smells like a race condition).

What is obvious looking at the code is that there is a db_delete() prior to db_insert(), but there is no lock for multiple requests.

iva2k’s picture

iva2k commented
Status: Active » Needs review
StatusFileSize
new session_restore_race_condition-517884-4.patch2.41 KB

The appropriate code pattern here would be using lock_acquire()/lock_wait()/lock_release(), as used for example in menu_build() in core.

Please review the attached patch that wraps delete/save operations on database {session_restore} table.

I tested it and had no problems/errors in the log doing a browser close-restore with about 20 tabs pointing to the website.