The ALGORITHM=INPLACE code in InnoDB assumes that the table is exclusively locked both at ha_innobase::prepare_inplace_alter_table() and at ha_innobase::commit_inplace_alter_table(). As a work-around for missing WL#6049 (proper metadata lock coverage for FOREIGN KEY operations), InnoDB does try to acquire exclusive transactional table locks at commit. And that is the reason why the ALTER TABLE operation goes to rollback:

		/* Exclusively lock the table, to ensure that no other

		transaction is holding locks on the table while we

		change the table definition. The MySQL meta-data lock

		should normally guarantee that no conflicting locks

		exist. However, FOREIGN KEY constraints checks and any

		transactions collected during crash recovery could be

		holding InnoDB locks only, not MySQL locks. */

		error = row_merge_lock_table(

			m_prebuilt->trx, ctx->old_table, LOCK_X);

		if (error != DB_SUCCESS) {

			my_error_innodb(

				error, table_share->table_name.str, 0);

			DBUG_RETURN(true);

		}

The above is the code in MariaDB 10.2. After the lock timeout, the SQL layer would invoke ha_innobase::commit_inplace_alter_table(commit=false), and during this operation, the assertion inside InnoDB would fail.

This seems to demonstrate poor error handling in InnoDB. The online_log is actually owned by the ALTER TABLE operation, and InnoDB should free it on failure, instead of complaining that it exists. A similar situation should be possible when the table being ALTERed is being locked as a child or parent table of a FOREIGN KEY operation.

I will try to create a test case for this. The original test case should be fixed as part of MDEV-15490 outside InnoDB.

I tested the FOREIGN KEY case, and there is no problem with that one: InnoDB will report an error after the exclusive lock acquisition failed at commit, and it would free the online_log as part of the rollback (below is the 10.2 version of the code):

	if (ctx->need_rebuild()) {

		dberr_t	err = DB_SUCCESS;

		ulint	flags	= ctx->new_table->flags;

		/* DML threads can access ctx->new_table via the

		online rebuild log. Free it first. */

		innobase_online_rebuild_log_free(prebuilt->table);

So, this bug seems to share a common root cause with MDEV-15490: XA transactions are not using meta-data locks correctly.
I successfully ran the following test in 10.0, 10.1, 10.2, 10.3:

--source include/have_innodb.inc

CREATE TABLE t1 (pk INT PRIMARY KEY) ENGINE=InnoDB;

CREATE TABLE t2 (pk INT PRIMARY KEY,

                 FOREIGN KEY(pk) REFERENCES t1(pk)) ENGINE=InnoDB;

--error ER_NO_REFERENCED_ROW_2

INSERT INTO t2 VALUES(1);

INSERT INTO t1 VALUES(1);

BEGIN;

INSERT INTO t2 VALUES(1);

--connect (con1,localhost,root,,test)

SET innodb_lock_wait_timeout= 1, lock_wait_timeout= 1;

--error ER_LOCK_WAIT_TIMEOUT

ALTER TABLE t1 FORCE, ALGORITHM=INPLACE;

--disconnect con1

--connection default

ROLLBACK;

DROP TABLE t2,t1;

Yes and no. Yes, metadata lock would've avoided this crash by preventing ha_innobase::commit_inplace_alter_table from being executed at all.

But the reason for this bug is different. It's a partitioned InnoDB table with two partitions. Online alter fails for some reason (deadlock in this particular case). The server needs to roll back the changes, it invokes ha_partition::commit_inplace_alter_table(commit=false). It invokes rollback_inplace_alter_table() for the first partition. It frees online log just fine, as you explained above. But then ha_partition invokes rollback_inplace_alter_table() for the second partition. Which does nothing because ctx->trx is NULL. And assert fires, as the online log for the second partition is not freed.

For partitioned tables, all changes should be committed or rolled back when invoking ha_innobase::commit_inplace_alter_table() on the first partition. The MySQL 5.6.11 change to implement this appears to be present in MariaDB 10.0.

In 10.0, the test case causes an invocation of ha_innobase::commit_inplace_alter_table(commit=true). This call will fail due to row_merge_lock_table() failing to acquire an exclusive transactional lock on the second partition (p1). Then we get subsequent calls to ha_innobase::commit_inplace_alter_table(commit=false), to roll back the ALTER TABLE operation (one call for each partition).

The error is caused because ha_innobase::commit_inplace_alter_table(commit=true) cleared the ctx->trx of other partitions than the first one, and rollback_inplace_alter_table() would skip some necessary steps.

It seems simplest to move the freeing of ctx->trx to a later stage. Alternatively, we could make rollback_inplace_alter_table() process all partitions of the table, instead of rolling back changes to each partition in separate commits.

I will write a DEBUG_SYNC test case so that the test will not have to be rewritten when the XA MDL bug is fixed. This should be repeatable by issuing a locking read on a partition while an online ALTER TABLE is running.

I wrote a DEBUG_SYNC test case, but it will always result in a MDL timeout, not InnoDB lock timeout. The InnoDB lock timeout can only be reached by exploiting bugs in the MDL. One bug would be the XA scenario in the bug report; another would be to exploit missing MDL coverage for certain FOREIGN KEY operations; however, FOREIGN KEY are not implemented for PARTITION tables.
For the record, here is my test, which fails to crash the server, because the innodb_lock_wait_timeout is not triggered:

--source include/have_innodb.inc

--source include/have_partition.inc

--source include/have_debug.inc

--source include/have_debug_sync.inc

CREATE TABLE t1 (a INT UNSIGNED PRIMARY KEY) ENGINE=InnoDB

PARTITION BY RANGE(a)

(PARTITION pa VALUES LESS THAN (3),

PARTITION pb VALUES LESS THAN (5));

INSERT INTO t1 VALUES(2),(4);

SET innodb_lock_wait_timeout= 1, lock_wait_timeout= 2;

SET DEBUG_SYNC = 'row_log_table_apply1_before SIGNAL ready WAIT_FOR locked';

send ALTER TABLE t1 FORCE;

connect dml,localhost,root,,;

SET DEBUG_SYNC = 'now WAIT_FOR ready';

BEGIN;

SELECT * FROM t1 WHERE a=4 LOCK IN SHARE MODE;

SET DEBUG_SYNC = 'now SIGNAL locked';

connection default;

--error ER_LOCK_WAIT_TIMEOUT

reap;

disconnect dml;

SET DEBUG_SYNC = 'RESET';

ALTER TABLE t1 FORCE;

DROP TABLE t1;

I also tried to trigger this with duplicate key error in a partitioned table, but it did not crash:

--source include/have_innodb.inc

--source include/have_partition.inc

--source include/have_debug.inc

--source include/have_debug_sync.inc

CREATE TABLE t1 (a INT, b INT) ENGINE=InnoDB

PARTITION BY RANGE(a)

(PARTITION pa VALUES LESS THAN (3),

PARTITION pb VALUES LESS THAN (5));

INSERT INTO t1 VALUES(2,2),(4,4);

SET innodb_lock_wait_timeout= 1, lock_wait_timeout= 2;

SET DEBUG_SYNC = 'inplace_after_index_build SIGNAL ready WAIT_FOR done';

send ALTER TABLE t1 ADD UNIQUE KEY(a,b), FORCE;

connect dml,localhost,root,,;

SET DEBUG_SYNC = 'now WAIT_FOR ready';

INSERT INTO t1 VALUES (4,4);

SET DEBUG_SYNC = 'now SIGNAL done';

disconnect dml;

connection default;

--error ER_DUP_ENTRY

reap;

SET DEBUG_SYNC = 'RESET';

DROP TABLE t1;