Re: [Maria-developers] [Commits] 93e746a: MDEV-7793 - Race condition between XA COMMIT/ROLLBACK and disconnect
Hi, Sergey! On Mar 17, svoj@mariadb.org wrote:
revision-id: 93e746afb2d1a23cb933291ad736a20858b5ac3e parent(s): ccc7297fe94af1129c717f91d31fa075d54a0371 committer: Sergey Vojtovich branch nick: mariadb timestamp: 2015-03-17 19:49:04 +0400 message:
MDEV-7793 - Race condition between XA COMMIT/ROLLBACK and disconnect
XA COMMIT/ROLLBACK of XA transaction owned by different thread may access freed memory if that thread disconnects at the same time.
Also concurrent XA COMMIT/ROLLBACK of recovered XA transaction were not serialized properly.
diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 3071ba6..f18231d 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -5355,7 +5378,7 @@ bool xid_cache_insert(THD *thd, XID_STATE *xid_state) switch (res) { case 0: - xid_state->xid_cache_element->mark_initialized(); + xid_state->xid_cache_element->set(XID_cache_element::ACQUIRED);
Why not mark_acquired() here? Can you be sure that no other thread has already acquired this xid_cache_element after you've inserted it?
break; case 1: my_error(ER_XAER_DUPID, MYF(0));
Regards, Sergei
Hi Sergei, On Thu, May 07, 2015 at 01:04:24PM +0200, Sergei Golubchik wrote:
Hi, Sergey!
On Mar 17, svoj@mariadb.org wrote:
revision-id: 93e746afb2d1a23cb933291ad736a20858b5ac3e parent(s): ccc7297fe94af1129c717f91d31fa075d54a0371 committer: Sergey Vojtovich branch nick: mariadb timestamp: 2015-03-17 19:49:04 +0400 message:
MDEV-7793 - Race condition between XA COMMIT/ROLLBACK and disconnect
XA COMMIT/ROLLBACK of XA transaction owned by different thread may access freed memory if that thread disconnects at the same time.
Also concurrent XA COMMIT/ROLLBACK of recovered XA transaction were not serialized properly.
diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 3071ba6..f18231d 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -5355,7 +5378,7 @@ bool xid_cache_insert(THD *thd, XID_STATE *xid_state) switch (res) { case 0: - xid_state->xid_cache_element->mark_initialized(); + xid_state->xid_cache_element->set(XID_cache_element::ACQUIRED);
Why not mark_acquired() here? mark_acquired() is intended to acquire RECOVERED elements. That is to allow multiple threads to attempt to acquire the same XID. One thread will succeed, others will get "not found".
Can you be sure that no other thread has already acquired this xid_cache_element after you've inserted it? Only RECOVERED elements can be acquired. This newly inserted element has m_state == 0.
Thanks, Sergey
Hi, Sergey! On May 07, Sergey Vojtovich wrote:
On Thu, May 07, 2015 at 01:04:24PM +0200, Sergei Golubchik wrote:
Hi, Sergey!
On Mar 17, svoj@mariadb.org wrote:
revision-id: 93e746afb2d1a23cb933291ad736a20858b5ac3e parent(s): ccc7297fe94af1129c717f91d31fa075d54a0371 committer: Sergey Vojtovich branch nick: mariadb timestamp: 2015-03-17 19:49:04 +0400 message:
MDEV-7793 - Race condition between XA COMMIT/ROLLBACK and disconnect
XA COMMIT/ROLLBACK of XA transaction owned by different thread may access freed memory if that thread disconnects at the same time.
Also concurrent XA COMMIT/ROLLBACK of recovered XA transaction were not serialized properly.
Why not mark_acquired() here? mark_acquired() is intended to acquire RECOVERED elements.
Ok to push, but please mention this in the method comment. Regards, Sergei
participants (2)
-
Sergei Golubchik
-
Sergey Vojtovich