Bug 71005 - File locking: No warning dialog is shown when started with --nolockcheck
Summary: File locking: No warning dialog is shown when started with --nolockcheck
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Extensions (show other bugs)
Version:
(earliest affected)
3.6.4.3 release
Hardware: x86 (IA32) Linux (All)
: medium normal
Assignee: Stephan Bergmann
URL:
Whiteboard: target:4.2.0
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-29 16:11 UTC by ulkitz
Modified: 2013-11-04 07:59 UTC (History)
1 user (show)

See Also:
Crash report or crash signature:


Attachments
possible patch, based on master (2.16 KB, patch)
2013-10-31 08:57 UTC, ulkitz
Details

Note You need to log in before you can comment on or make changes to this bug.
Description ulkitz 2013-10-29 16:11:16 UTC
When two users from different computers open the same file, normally the second user gets a warning "Document is locked for editing by ...". This dialog is not shown when office is started with parameter --nolockcheck.

On the other hand, a Java application which uses the com.sun.star.comp.helper.Bootstrap class starts Libreoffice with --nolockcheck. In the end, when a such a Java application runs in the background (like the WollMux at the city of Munich), and a user opens a document which is locked by another user, he gets no warning dialog.

I think it's because the CommandLineArgs:InterpretCommandLineParameter method uses the nolockcheck-parameter as a workaround for automated tests, as the comment says. But that's not what nolockcheck is said to be used for in the parameter descriptions, "--nolockcheck  don't check for remote instances using the installation". So, if it's still neccessary to supress the warning dialog for automated tests I think it would be a good idea to introduce a new parameter.

I created this bug for 3.6.4, but I think it's still the same with at least 4.1.3
Comment 1 ulkitz 2013-10-31 08:57:07 UTC
Created attachment 88402 [details]
possible patch, based on master
Comment 2 Stephan Bergmann 2013-10-31 10:01:22 UTC
Background:  No two concurrently running instances of LO must use the same UserInstallation, or else data corruption can occur.  On a single machine, this is taken care of by the "OfficeIPC" protocol.  For the case that UserInstallation is on a file system accessible from multiple machines, this is taken core of by a .lock file in the UserInstallation.  However, when a LO instance crashes and leaves behind a stale .lock file, new LO instances (from other machines, at least) could not start.  Therefore, when a LO instance sees a potentially stale .lock file written from another machine: (a) if --nolockcheck is not given, it asks the user whether the file is indeed stale (and proceeds only if the user confirms it is stale); (b) if --nolockcheck is given, silently assume the file is indeed stale and proceeds (potentially causing data corruption if the assumption was false).

The

> // Workaround for automated testing
> ::svt::DocumentLockFile::AllowInteraction( false );

in case of --nolockcheck has been added by <http://cgit.freedesktop.org/libreoffice/core/commit/?id=62ee5e1a752033344c172ad2380a5f1e2492330a> "INTEGRATION: CWS calcshare2: #i85794# workaround for automated testing" in the context of <https://issues.apache.org/ooo/show_bug.cgi?id=85794> "Reimplement file locking using an additional file."  I see no good reason for that, esp. as all our (subsequent-)checks each use a dedicated, throwaway UserInstallation anyway.  So I'm inclined to just remove those two lines (and not add an additonal --autotest argument as suggested by attachment 88402 [details]).

(Whether it is actually good that LO is called with --nolockcheck in cppuhelper/source/bootstrap.cxx, extensions/source/nsplugin/source/so_main.cxx, and javaunohelper/com/sun/star/comp/helper/Bootstrap.java, potentially causing data corruption, is a different matter.)
Comment 3 Commit Notification 2013-11-01 10:17:14 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2c2ff538908e4c61987469c9e09325c2b479b185

fdo#71005 Remove odd "Workaround for automated testing"



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 4 Stephan Bergmann 2013-11-01 10:21:53 UTC
ukitz, any demand for a backport to LO 4.1.4?
Comment 5 ulkitz 2013-11-04 07:59:10 UTC
No, for us it's not necessary to backport that fix.