On Sat, Jul 07, 2007 at 09:27:33PM +0200, Xavier wrote:
On Fri, Jul 06, 2007 at 11:29:55PM +0200, Xavier wrote:
There is a second problem (still unusual case) that I need to figure out : when first installing a package as root, then installing a package as user, I get the error and behavior as described here : http://www.archlinux.org/pipermail/pacman-dev/2007-April/008008.html ie pacman freezes and can't even be cancelled with ctrl+C.
It would be nicer if it exited cleanly instead :)
The problem is that when a transaction fails, trans->state isn't set to STATE_COMMITED, but stays on STATE_COMMITING (see _alpm_trans_commit function in libalm/trans.c). Then when _alpm_trans_release is called, it does the following :
195 /* during a commit do not interrupt immediately, just after a target */ 196 if(trans->state == STATE_COMMITING || trans->state == STATE_INTERRUPTED) { 197 if(trans->state == STATE_COMMITING) { 198 trans->state = STATE_INTERRUPTED; 199 } 200 pm_errno = PM_ERR_TRANS_COMMITING; 201 return(-1); 202 }
Does anyone understand the comment above? Anyway, now the state is set to INTERRUPTED. And the alpm_release function in libalpm/alpm.c :
77 while((dbs_left = alpm_list_count(handle->dbs_sync)) > 0) { 78 pmdb_t *db = (pmdb_t *)handle->dbs_sync->data; 79 _alpm_log(PM_LOG_DEBUG, _("removing DB %s, %d remaining..."), db->treename, 80 alpm_db_unregister(db); 81 db = NULL; 82 }
There is an infinite loop there when alpm_db_unregister (db.c) fails, and it fails when handle->trans isn't NULL : 86 /* Do not unregister a database if a transaction is on-going */ 87 ASSERT(handle->trans == NULL, RET_ERR(PM_ERR_TRANS_NOT_NULL, -1));
Could we consider that when trans is not NULL, but trans->state is set to STATE_INTERRUPTED, then it isn't a on-going transaction anymore, and so we can unregister the database safely ?
From b130e08ed216e3a9caa0026bec213d9a0a5fa094 Mon Sep 17 00:00:00 2001 From: Chantry Xavier <shiningxc@gmail.com> Date: Sat, 7 Jul 2007 21:24:30 +0200 Subject: [PATCH] libalpm/db.c : allow unregistering db for interrupted transaction.
This prevents alpm_release to loop infinitely in case of an interrupted transaction, where the database wasn't unregistered. alpm_release should probably also be fixed, as it can still loop if db_unregister fails for another reason.
Signed-off-by: Chantry Xavier <shiningxc@gmail.com> --- lib/libalpm/db.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c index 95a1ecb..7cc9390 100644 --- a/lib/libalpm/db.c +++ b/lib/libalpm/db.c @@ -84,7 +84,8 @@ int SYMEXPORT alpm_db_unregister(pmdb_t *db) ASSERT(handle != NULL, RET_ERR(PM_ERR_HANDLE_NULL, -1)); ASSERT(db != NULL, RET_ERR(PM_ERR_WRONG_ARGS, -1)); /* Do not unregister a database if a transaction is on-going */ - ASSERT(handle->trans == NULL, RET_ERR(PM_ERR_TRANS_NOT_NULL, -1)); + ASSERT(handle->trans == NULL || handle->trans->state == STATE_INTERRUPTED, + RET_ERR(PM_ERR_TRANS_NOT_NULL, -1));
if(db == handle->db_local) { handle->db_local = NULL; -- 1.5.2.2
While looking at FS#7652, I found something new : this problem doesn't happen on -S operations, only on -U, because of the following code in sync.c , _alpm_sync_commit function : 1027 error: 1028 _alpm_trans_free(tr); 1029 tr = NULL; 1030 /* commiting failed, so this is still just a prepared transaction */ 1031 trans->state = STATE_PREPARED; 1032 return(-1); So if an error happens in sync code, the state goes back to STATE_PREPARED. That looks even more hackish than my patch above, but if it's better, then the same should be done in _alpm_add_commit and _alpm_remove_commit. Otherwise, it should be removed and handled somewhere else, like with my patch. Which way should we take?