Fix crash copying, pasting, and transforming selection (fix #4249)

Regression introduced with 8722c8ec16
and d3562b140c.

Our re-entrant RWLock implementation is tricky because it doesn't
support to unlock in different order than it was locked.

E.g. If we create a DocWriter (e.g. paste command), and try to create
a DocReader (e.g. PixelsMovement) inside the DocWriter scope, the
reader will return a LockResult::Reentrant, but if we unlock the
write, then the reader cannot be used to upgradeToWrite() because the
lock will have just 0 locks (the re-entrant one didn't modify the
count of the RWLock). This is because it was thought that the
DocReader was going to be unlocked before the writer lock (in the
exact inverse order).

Basically these re-entrant RWLocks will not work fine if we mix up the
order of locks/unlocks in the same thread.
This commit is contained in:
David Capello 2023-12-29 17:11:44 -03:00
parent d93655647d
commit f178941f2c
5 changed files with 14 additions and 12 deletions

View File

@ -256,9 +256,8 @@ void NewLayerCommand::onExecute(Context* context)
}
#endif
ContextWriter writer(reader);
LayerGroup* parent = sprite->root();
Layer* activeLayer = writer.layer();
Layer* activeLayer = reader.layer();
SelectedLayers selLayers = site.selectedLayers();
if (activeLayer) {
if (activeLayer->isGroup() &&
@ -274,6 +273,7 @@ void NewLayerCommand::onExecute(Context* context)
Layer* layer = nullptr;
{
ContextWriter writer(reader);
Tx tx(writer,
fmt::format(Strings::commands_NewLayer(), layerPrefix()));
DocApi api = document->getApi(tx);
@ -427,7 +427,7 @@ void NewLayerCommand::onExecute(Context* context)
#ifdef ENABLE_UI
// Paste new layer from clipboard
else if (params().fromClipboard() && layer->isImage()) {
context->clipboard()->paste(writer, false);
context->clipboard()->paste(context, false);
if (layer->isReference()) {
if (Cel* cel = layer->cel(site.frame())) {

View File

@ -1,5 +1,5 @@
// Aseprite
// Copyright (C) 2018-2022 Igara Studio S.A.
// Copyright (C) 2018-2023 Igara Studio S.A.
// Copyright (C) 2001-2018 David Capello
//
// This program is distributed under the terms of
@ -594,8 +594,7 @@ bool DocView::onPaste(Context* ctx)
auto clipboard = ctx->clipboard();
if (clipboard->format() == ClipboardFormat::Image ||
clipboard->format() == ClipboardFormat::Tilemap) {
ContextWriter writer(ctx);
clipboard->paste(writer, true);
clipboard->paste(ctx, true);
return true;
}
else

View File

@ -4391,8 +4391,7 @@ bool Timeline::onPaste(Context* ctx)
invalidate();
}
ContextWriter writer(ctx);
clipboard->paste(writer, true);
clipboard->paste(ctx, true);
return true;
}
else

View File

@ -461,10 +461,10 @@ void Clipboard::copyPalette(const Palette* palette,
m_data->picks = picks;
}
void Clipboard::paste(ContextWriter& writer,
void Clipboard::paste(Context* ctx,
const bool interactive)
{
const Site site = *writer.site();
const Site site = ctx->activeSite();
Doc* dstDoc = site.document();
if (!dstDoc)
return;
@ -531,6 +531,7 @@ void Clipboard::paste(ContextWriter& writer,
if (!dstLayer || !dstLayer->isImage())
return;
ContextWriter writer(ctx);
Tx tx(writer, "Paste Image");
DocApi api = dstDoc->getApi(tx);
Cel* dstCel = api.addCel(
@ -611,6 +612,7 @@ void Clipboard::paste(ContextWriter& writer,
break;
}
ContextWriter writer(ctx);
Tx tx(writer, "Paste Cels");
DocApi api = dstDoc->getApi(tx);
@ -671,6 +673,7 @@ void Clipboard::paste(ContextWriter& writer,
break;
}
ContextWriter writer(ctx);
Tx tx(writer, "Paste Frames");
DocApi api = dstDoc->getApi(tx);
@ -714,6 +717,7 @@ void Clipboard::paste(ContextWriter& writer,
if (srcDoc->colorMode() != dstDoc->colorMode())
throw std::runtime_error("You cannot copy layers of document with different color modes");
ContextWriter writer(ctx);
Tx tx(writer, "Paste Layers");
DocApi api = dstDoc->getApi(tx);

View File

@ -1,5 +1,5 @@
// Aseprite
// Copyright (C) 2019-2021 Igara Studio S.A.
// Copyright (C) 2019-2023 Igara Studio S.A.
// Copyright (C) 2001-2018 David Capello
//
// This program is distributed under the terms of
@ -74,7 +74,7 @@ namespace app {
const doc::Tileset* tileset);
void copyPalette(const doc::Palette* palette,
const doc::PalettePicks& picks);
void paste(ContextWriter& writer,
void paste(Context* ctx,
const bool interactive);
doc::ImageRef getImage(doc::Palette* palette);