From Bugzilla Helper:
User-Agent: Mozilla/5.0 Galeon/1.2.7 (X11; Linux i686; U;) Gecko/20030131
Description of problem:
The unpublish() method loops through all pending versions and removes them from
the draft version. However, it does not delete them. A pending version with no
draft version has no use, and just takes up space in the DB. It also violates
the "com.arsdigita.cms.dbinvariants.pendingLiveMarkedMap" invariant. The
removePendingVersion() method should be deleting the pending versions after
removing them from the draft version.
Version-Release number of selected component (if applicable):
Troika 6.0.0
How reproducible:
Always
Steps to Reproduce:
1. Create a new ContentItem.
2. Apply a lifecycle with a start date of tomorrow.
3. Call unpublish() on it.
Actual Results: The pending version created in step 2 remains in the database.
Expected Results: The pending version should have been deleted.
Additional info:

Changed this to a high priority bug because it's impacting a client right now.
During their import process they end up publishing an item multiple times. This
currently results in multiple pending versions of items getting orphaned in the
DB, which is problematic. I'm going to have to patch this for them very soon,
and I'd like to know what the correct solution is. I feel like the right thing
is for removeVersion() to delete the removed version, but I'd like the
engineering perspective on it. Please look into this ASAP.

Mike,
It seems like deleting the pending version after it has been removed
from the draft is the right thing to do. (Provided we don't run into
DB constraint violations. It wouldn't be surprising, if we did.)
While we are on this subject, I'd like to ask for your opinion on
making the removePendingVersion method protected, instead of public.
ContentItem only uses it in the unpublish() method.
(Self-use of public methods is generally a bad idea, 'cause you
don't really know what you are calling. You may *think* you are
calling the removePendingVersion method of the ContentItem class,
while in reality, due to polymorphic method dispatch, you are
actually calling the removePendingVersion method of the most
specific subclass, which may have been overridden in a dumb way
that violates your assumptions.)
Folder and ContentBundle basically override this method to do nothing.
Folder
public void removePendingVersion(final ContentItem version) {
Assert.unequal(PENDING, version.getVersion());
return;
}
ContentItem only uses it in unpublish()
ArticleImageChooser.java uses it in the deleteImage method. It removes
pending versions of the asset, before deleting the asset. This logic
can be moved into the Asset class, which would have access to the
protected removePendingVersion in the superclass.
So, based on current usage of this method, I see no reason for this
method to be public.
Thoughts?
I will get to this by the end of tomorrow, at the latest.

Created attachment 94178[details]
Proposed patch for deleting the pending/live version after removal.
I think this is the correct solution, but it is totally untested at this point.
The client will be testing it rigorously later today, at which point I may
modify the patch.

Created attachment 94196[details]
New patch to delete pending/live versions when they are removed from the draft version.
This patch tries to address deadlock conditions that were occurring with the
previous patch. It removes a deadlock that was occurring when
ContentBundle.beforeSave() was being called during the delete (apparently
related to bz 103022). Deadlocks are still occurring between the deleting
thread and the Lifecycle thread. Always removing the lifecycle before deleting
the pending version may resolve this, but I haven't had time to implement/test
it yet.

Mike,
Can you explain the deadlock issue in more detail?
At this point, the wisest course of action would seem to
be to let you guys test your patch out a little more,
before I merge it onto the trunk.
Seeing the words "patch" and "deadlock" in the same sentence
makes me nervous.

When I integrated the publishing changes at 36166, I added a call to delete() in
removePendingVersion(). My code doesn't deal with the deadlock issues (which may
or may not still be an issue with more recent CMS changes), so Mike's patch or
some variant may still be needed. The relevant portion of the diff for 36166 is:
930a934
> version.delete();
In addition, we need some code to clean up the current orphaned pending
versions. Nuno ran into a support issue where removed (but not deleted) pending
versions no longer show up in the UI (as the connection to the draft item was
severed), but they are still in the live folder, preventing the folder from
being deleted or moved. We need something along the lines of:
1) retrieve all on ContentItem.
2) Filter on "version='pending'" and "masterVersion is null"
3) delete these items, unpublishing the folder/bundle/etc. if appropriate