Bug 133898 - Call to loadCalendarTZ crashes LibreOffice
Summary: Call to loadCalendarTZ crashes LibreOffice
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: sdk (show other bugs)
Version:
(earliest affected)
6.4.3.2 release
Hardware: x86-64 (AMD64) All
: medium normal
Assignee: Julien Nabet
URL:
Whiteboard: target:7.1.0 target:7.0.0.1 target:6.4.6
Keywords: haveBacktrace
Depends on:
Blocks:
 
Reported: 2020-06-11 12:45 UTC by libre officer
Modified: 2020-06-16 13:16 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
bt with debug symbols (14.00 KB, text/plain)
2020-06-11 14:17 UTC, Julien Nabet
Details
bt2 (12.66 KB, text/plain)
2020-06-11 14:32 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description libre officer 2020-06-11 12:45:23 UTC
Description:
Call to loadCalendarTZ crashes LibreOffice.

Sometimes, crash and run indefinitely (need to be killed manually by the process manager).

Steps to Reproduce:
1. Run this code from the Basic window (no document is opened)

Sub Main
	dim oLocale As New com.sun.star.lang.Locale
	oLocale.Language = "ar"

	oCal = CreateUnoService("com.sun.star.i18n.Calendar_gregorian")	
	oCal.loadCalendarTZ("gregorian", oLocale, "UTC")
	
End Sub

Actual Results:
LibreOffice crashes.

Expected Results:
load the specified calendar.


Reproducible: Always


User Profile Reset: No



Additional Info:
Version: 7.0.0.0.beta1 (x64)
Build ID: 94f789cbb33335b4a511c319542c7bdc31ff3b3c
CPU threads: 4; OS: Windows 10.0 Build 17763; UI render: Skia/Raster; VCL: win
Locale: fr-CH (fr_FR); UI: en-GB
Calc: threaded


https://crashreport.libreoffice.org/stats/crash_details/930a6b35-9d70-4336-8f64-42b2f1a4b51d
Comment 1 Julien Nabet 2020-06-11 14:17:03 UTC
Created attachment 161882 [details]
bt with debug symbols

On pc Debian x86-64 with master sources updated today, I got an assertion but in a non debug build, I suppose it'll crash indeed.
Comment 2 Julien Nabet 2020-06-11 14:18:16 UTC
Mike: thought you might be interested in this one.
The pb here is we got no context.
(gdb) frame 5
#5  0x00007fffe37c2283 in i18npool::CalendarImpl::loadCalendarTZ (this=0x890ec80, uniqueID="gregorian", rLocale=..., rTimeZone="UTC") at i18npool/source/calendar/calendarImpl.cxx:70
70	        Reference < XInterface > xI = m_xContext->getServiceManager()->createInstanceWithContext(
(gdb) list
65	            break;
66	        }
67	    }
68	
69	    if (i >= sal::static_int_cast<sal_Int32>(lookupTable.size())) {
70	        Reference < XInterface > xI = m_xContext->getServiceManager()->createInstanceWithContext(
71	                  "com.sun.star.i18n.Calendar_" + uniqueID, m_xContext);
72	
73	        if ( ! xI.is() ) {
74	            // check if the calendar is defined in localedata, load gregorian calendar service.
(gdb) p m_xContext
$1 = empty uno::Reference

(I just launched Writer, copy pasted the code from the author)
Comment 3 Julien Nabet 2020-06-11 14:32:38 UTC
Created attachment 161883 [details]
bt2

Here's the bt where ctr without arg (so with no context) is called.
#0  i18npool::CalendarImpl::CalendarImpl() (this=0x887cc20) at i18npool/inc/calendarImpl.hxx:42
#1  0x00007fffe27002cc in i18npool::Calendar_gregorian::Calendar_gregorian() (this=0x887cc20) at i18npool/source/calendar/calendar_gregorian.cxx:132
#2  0x00007fffe2778f0e in Calendar_gregorian_CreateInstance(com::sun::star::uno::Reference<com::sun::star::lang::XMultiServiceFactory> const&) () at i18npool/source/registerservices/registerservices.cxx:113
#3  0x00007ffff6733f81 in cppu::(anonymous namespace)::OSingleFactoryHelper::createInstanceEveryTime(com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&) (this=0x85f4238, xContext=
    uno::Reference to (cppu::(anonymous namespace)::ComponentContext *) 0x54dab0) at cppuhelper/source/factory.cxx:151
#4  0x00007ffff67345e7 in cppu::(anonymous namespace)::OSingleFactoryHelper::createInstanceWithContext(com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&)
    (this=0x85f4238, xContext=uno::Reference to (cppu::(anonymous namespace)::ComponentContext *) 0x54dab0) at cppuhelper/source/factory.cxx:177
...
Comment 4 Mike Kaganski 2020-06-11 15:19:06 UTC
(In reply to Julien Nabet from comment #2)
> The pb here is we got no context.

Try introducing a local variable for context, assign it to m_xContext if it's valid, or to comphelper::getProcessComponentContext() otherwise; and use this local variable in place of m_xContext there?
Comment 5 Julien Nabet 2020-06-11 15:28:46 UTC
(In reply to Mike Kaganski from comment #4)
> (In reply to Julien Nabet from comment #2)
> > The pb here is we got no context.
> 
> Try introducing a local variable for context, assign it to m_xContext if
> it's valid, or to comphelper::getProcessComponentContext() otherwise; and
> use this local variable in place of m_xContext there?

What about using comphelper::getProcessComponentContext() in the ctr where context isn't provided by arg?
Also for the second ctr, if context provided by arg is empty, I'd use comphelper::getProcessComponentContext() too, what do you think?

if you don't want ctrs be modified, I could change this block:
     70     if (i >= sal::static_int_cast<sal_Int32>(lookupTable.size())) {
     71         Reference < XInterface > xI = m_xContext->getServiceManager()->createInstanceWithContext(
     72                   "com.sun.star.i18n.Calendar_" + uniqueID, m_xContext);
     73 
     74         if ( ! xI.is() ) {
     75             // check if the calendar is defined in localedata, load gregorian calendar service.
     76             const Sequence< Calendar2 > xC = LocaleDataImpl::get()->getAllCalendars2(rLocale);
     77             if (std::any_of(xC.begin(), xC.end(), [&uniqueID](const Calendar2& rCal) { return uniqueID == rCal.Name; }))
     78                 xI = m_xContext->getServiceManager()->createInstanceWithContext("com.sun.star.i18n.Calendar_gregorian", m_xContext);
     79         }

by:
     70     if (i >= sal::static_int_cast<sal_Int32>(lookupTable.size())) {
++                if (!m_xContext.is())
++                    m_xContext = comphelper::getProcessComponentContext();
     71         Reference < XInterface > xI = m_xContext->getServiceManager()->createInstanceWithContext(
     72                   "com.sun.star.i18n.Calendar_" + uniqueID, m_xContext);
     73 
     74         if ( ! xI.is() ) {
     75             // check if the calendar is defined in localedata, load gregorian calendar service.
     76             const Sequence< Calendar2 > xC = LocaleDataImpl::get()->getAllCalendars2(rLocale);
     77             if (std::any_of(xC.begin(), xC.end(), [&uniqueID](const Calendar2& rCal) { return uniqueID == rCal.Name; }))
     78                 xI = m_xContext->getServiceManager()->createInstanceWithContext("com.sun.star.i18n.Calendar_gregorian", m_xContext);
     79         }
Comment 6 Mike Kaganski 2020-06-11 15:34:49 UTC
(In reply to Julien Nabet from comment #5)

Your approach with modifying default ctor looks really superior to mine. Please to it. (But I wouldn't modify the second one, or better throw from it if context is empty. Using it and not passing a valid context there is an error.)
Comment 7 Mike Kaganski 2020-06-11 15:36:25 UTC
(In reply to Mike Kaganski from comment #6)

... but please move the default ctor implementation into CXX, to avoid introducing new dependencies on additional header to all places that use the class.
Comment 8 Julien Nabet 2020-06-11 17:08:24 UTC
Patch waiting for review here:
https://gerrit.libreoffice.org/c/core/+/96161
Comment 9 Commit Notification 2020-06-12 07:40:12 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/ff508f6d8a3e58d29e9e7622006a7103fb0a2849

tdf#133898: get a context in default ctr (i18npool/calendarImpl)

It will be available in 7.1.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 10 Julien Nabet 2020-06-12 07:50:03 UTC
Patch for branch 7.0 waiting here:
https://gerrit.libreoffice.org/c/core/+/96161
Comment 11 Xisco Faulí 2020-06-16 11:13:24 UTC
Verified in

Version: 7.1.0.0.alpha0+
Build ID: 11d21b3c1f7754b5d13ae9ea88da562ec74366ff
CPU threads: 4; OS: Linux 4.19; UI render: default; VCL: x11
Locale: en-US (en_US.UTF-8); UI: en-US
Calc: threaded

@Julien Nabet, thanks for fixing this issue!!
Comment 12 Commit Notification 2020-06-16 11:14:52 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-7-0":

https://git.libreoffice.org/core/commit/bca5d96335fd3742b721e99b10934502b105259a

tdf#133898: get a context in default ctr (i18npool/calendarImpl)

It will be available in 7.0.0.1.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 13 Commit Notification 2020-06-16 13:16:53 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-6-4":

https://git.libreoffice.org/core/commit/c1638c1012593f6d6336a9d94c73fb074fed6a47

tdf#133898: get a context in default ctr (i18npool/calendarImpl)

It will be available in 6.4.6.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.