Look for places where we are accidentally assigning a returned-by-value
VclPtr<T> to a T*, which generally ends up in a use-after-free.
Change-Id: I4f361eaca88820cdb7aa3b8340212db61580fdd9
Reviewed-on: https://gerrit.libreoffice.org/30749
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
...which was introduced with 3ead3ad52f9bb2f9d1d6cf8dfc73a0a25e6778ed "Gradually
typed Link" to distinguish the new, typed versions from the old, untyped ones,
but is no longer necessary since 382eb1a23c390154619c385414bdbe6f6e461173
"remove untyped Link<>" removed the old versions.
Change-Id: I494025df486a16a45861fcd8192dfe0275b1103c
...otherwise, if $(username) happens to be "user", it will endlessly re-
substitute inside a (already partly re-substituted) URL of the form
$(user)/...
Regression introduced with 16fb0d3d0f68708c183c53bd18660a23970b77fe "tdf#98407
PathSubstitution: Add substitution for $(username)".
Change-Id: I1c8b64f383fdfd97fa5edc192e9ca4b46944d6f1
AutoRecovery::implts_updateModifiedState() should call external
functions like isModified() before acquiring its own mutex.
(regression from 403eefe81b8a0afe888c60452c17d6b2c5d8343f)
Change-Id: Iae56eec2b6f392b7a7f65a5f54c73efa746263d0
The issue of 362d4f0cd4e50111edfae9d30c90602c37ed65a2 "Explicitly mark
overriding destructors as 'virtual'" appears to no longer be a problem with
MSVC 2013.
(The little change in the rewriting code of compilerplugins/clang/override.cxx
was necessary to prevent an endless loop when adding "override" to
OOO_DLLPUBLIC_CHARTTOOLS virtual ~CloseableLifeTimeManager();
in chart2/source/inc/LifeTime.hxx, getting stuck in the leading
OOO_DLLPUBLIC_CHARTTOOLS macro. Can't remember what that
isAtEndOfImmediateMacroExpansion thing was originally necessary for, anyway.)
Change-Id: I534c634504d7216b9bb632c2775c04eaf27e927e
The auto-recovery service maintains a list of structures (one for each open
document) containing information needed to carry out the auto-save
functionality. One such piece of information is the location of the backup
file, stored in a struct member named 'OldTempURL'. At every auto-save
interval, this list is iterated through and a function (implts_saveOneDoc)
is called during each iteration to save the current state of the associated
document.
The algorithm works as follows:
1. A new backup file URL is chosen so as not to conflict with any already
existing backup files in the backup directory. This URL is based on the
file name and incorporates a number (starting at 0) that is incremented
until a name is chosen that doesn't conflict.
2. The document is saved to this new backup file URL
3. The previous backup file (indicated by its structure's 'OldTempURL') is
deleted
4. The new backup file URL is stored (in its structure's 'OldTempURL') for the
next time the file needs to be saved.
Assuming you start with a new Writer doc and then make some changes, when it is
time to auto-save, the backup file name 'untitled_0.odt' (excluding path) will
be selected, the latest state of the open file will be written to that backup
file, and the full URL for the backup file will be saved into the struct
'OldTempURL' member.
The next time changes are made and an auto-save occurs, this algorithm will
result in the name 'untitled_1.odt' being selected, the file contents saved
into this new file, 'untitled_0.odt' being deleted, and the full URL for the
new backup file being saved in 'OldTempURL'.
The third time through results in 'untitled_0.odt' being selected (since this
file doesn't exist on disk), and subsequent iterations of auto-saving cause
the backup file name to alternate between the two aforementioned.
The problem occurs during a 'Save as' operation. When this happens, the backup
file is deleted (which is fine - it was just saved, and the next auto-save will
back it up) but 'OldTempURL' is not properly reset (see below for more info.)
During the next auto-save, 'untitled_0.odt' will be selected for the new backup
file name (since no file exists by this name), and one of two things will
happen (based on how many auto-saves have occurred):
1. 'OldTempURL' points to 'untitled_1.odt', and the algorithm above continues
to work correctly (at least in that it continues to backup file contents.)
2. 'OldTempURL' points to 'untitled_0.odt', the name chosen for the new backup
file. In this case, the document contents will be saved to this file
(step 2) but then the file will be deleted (step 3). 'OldTempURL' will
maintain this URL from then on out, causing this case to be hit for all
future auto-save intervals.
So, 50% of the time (30 minutes out of every hour) auto-save will stop backing
up file contents on a 'Save as'.
The function that handles the 'Save as' case (implts_markDocumentAsSaved)
clears 'OldTempURL' and sets other relavent struct members for a local variable
copy of the global struct, but doesn't copy them back. :( These changes are
effectively lost when the function returns.
There are several other cases where this appears to be happening as well, but
more work is needed to determine whether this is actually the case:
- implts_prepareSessionShutdown
- implts_saveDocs, handling the 'dangerousDocs' and in a few other places
- implts_openDocs
- implts_resetHandleStates
Also, there is some JUnitTest code for auto-save, but it is currently disabled
(and fails to run successfully.) It'd be great to get these working again, or
to just write python equivalents. Implementing this would like take me a while,
though, so for now I just tested manually to ensure that this fixes the issue.
When I have some more time I'd like to work more on this, but I wanted to send
this patch in for now to address bug #96607.
This may also address bug #99890, since some of the struct members that don't
make it into the global state relate to the file name. I haven't explicitly
tested this case, though.
Change-Id: Ic702d6f78e60c7cf828a1564ccca118dd45d152b
Reviewed-on: https://gerrit.libreoffice.org/25948
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: jan iversen <jani@documentfoundation.org>
but stop basic execution on the exit attempt, and then resend exit at a safe
place when basic execution has stopped
Change-Id: I77c43acffa0b82e8125dcb3b10ad9bf0d6dd26c3
- move DispatchHelper somewhere public
- use it from generic dispatcher call sites in sfx2
- return result of dispatcher calls (conveyed via
XDispatchResultListener) to calling code, instead of faking it
Change-Id: Ie8041133e99dd99e45819f98798829b96532b9e6
Reviewed-on: https://gerrit.libreoffice.org/24953
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Thorsten Behrens <Thorsten.Behrens@CIB.de>
probably not much performance benefit, but it sure is good at
identifying leftover intermediate variables from previous
refactorings.
Change-Id: I3ce16fe496ac2733c1cb0a35f74c0fc9193cc657
Reviewed-on: https://gerrit.libreoffice.org/24026
Reviewed-by: Noel Grandin <noelgrandin@gmail.com>
Tested-by: Noel Grandin <noelgrandin@gmail.com>
What a mess. Ideally, Frame would use its own rBHelper.rMutex, not SolarMutex.
But much of the framework code it calls into uses SolarMutex, too, making it
difficult to change that without running into the risk of deadlock. And then,
some member variables are cleared early in Frame::disposing, while others are
only cleared en bloc at the end. Be conservative and keep it that way (as other
Frame functions recursively called from within Frame::disposing could observe
the difference and rely on the current behavior), even if that means creating
lots of small, independent locked regions within Frame::disposing (which can be
detrimental to both performance and correctness).
Change-Id: I28f9a379ce03ed661e96c7deb8eb73cb58fb2cf7
... in modules editeng to oox.
Replace with C++11 delete copy-constructur and
copy-assignment.
Remove boost/noncopyable.hpp includes and
one unused boost/checked_delete.hpp include in linguistic.
Change-Id: I5a38d8e5ac1b4286bdeb3858d56490a53d13fe80
Reviewed-on: https://gerrit.libreoffice.org/23928
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noelgrandin@gmail.com>
...to get the deadlock under control the fix for which had to be reverted with
22fbbfe984f5da2592674f9260f5e4988b1341fe "Revert 'Avoid deadlock when two
threads call into Frame::close'".
Generally, replaced instances of TransactionGuard(E_HARDEXCEPTIONS) with
checkDisposed() and instances of TransactionGuard(E_SOFTEXCEPTIONS) with
nothing. A TransactionGuard would not establish a locked section per se (only
would it engage in locking when TransactionManager::setWorkingMode is called,
see the deadlock mentioned above), so the potential for introducing new races
should be manageable.
While at it, get rid of those implcp_* "debug methods" used to SAL_WARN about
bad arguments to some UNO interface method implementations.
Change-Id: I5ea9c1c8b20fd38457c558dbcb3a853a51a09b6e
It originally got introduced with b1da5a57d93e8e9b43b9bba9fabc3b7e61289edc
"CWS-TOOLING: integrate CWS alf01" but appears to have never been used.
Curiously, there were two commits to its code
(f565a4b3d9b47ca3336df4f8d8d56d4e2dcceec5 "#i107087# TabWindowService must set
member pointer to zero on VCLEVENT_OBJECT_DYING. The window life time is
controlled by the docking window! Adding missing RemoveEventListener call" and
84c0ebe7d3e7dbc3796967d52f9535fecc9e6947 "#i107087# Remove event listener from
tab window on TabWindowService dtor") that mention
<https://bz.apache.org/ooo/show_bug.cgi?id=107087> "button merged in standard
toolbar + own UI caused office crash", but the description of which ("when i
merge a toolbar button in a standard toolbar and provide own UI (via the
toolkit) the office crashed sometimes") doesn't give a hint how this service
would have been used. (Maybe it was intended to be used by extensions, but
never got documented neither in offapi's UNOIDL nor in
<https://wiki.openoffice.org/wiki/Documentation/DevGuide/OpenOffice.org_Developers_Guide>.)
Change-Id: I86746f73ee99ebe7f5afbc902204a9353e5ccb7b
