Bug 121342 - Access2Base Counter for OpenRecordset usage is too low
Summary: Access2Base Counter for OpenRecordset usage is too low
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
6.1.1.1 rc
Hardware: All All
: medium enhancement
Assignee: Jean-Pierre Ledure
URL:
Whiteboard: target:6.2.0
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-11 03:21 UTC by tsultana
Modified: 2018-11-16 08:49 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
OpenRecordset error after opening and closing 32767 Recordset objects (8.50 KB, image/png)
2018-11-11 03:22 UTC, tsultana
Details
Recordset.xba test (44.78 KB, text/html)
2018-11-11 19:56 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description tsultana 2018-11-11 03:21:20 UTC
Description:
Each use of OpenRecordset increases the Integer counter RecordsetMax in Access2Base.Database.  Once 32767 recordsets are open an error is reported with the next OpenRecordset.  It does not matter that mClose() is used for each OpenRecordset.

32767 may sound large but my use is a converted MS Access database used for Monte Carlo simulations into the millions.  I am reworking the logic in many places to use arrays or batch updates with difficulty.

The following code will error out very quickly.

Dim iRec as Object, x as Long
Set iRec = CurrentDb().OpenRecordset("table")
With iRec 
  Do While Not .EOF
    x = x + 1
    .MoveNext
  Loop
  .mClose
End With 



Steps to Reproduce:
1. Run the code above to get the attached error.
2.
3.

Actual Results:
Code runs until the RecordsetMax gets to 32767.  Then either 
1) run CurrentDb().CloseAllRecordsets() to reset but make sure you are not using any open Recordsets or
2) Close all LO products and reopen Base

This problem is on Debian Linux LO 6.1.3 and Win10 LO 6.1.1

Expected Results:
Crashed of OpenRecordset with FATAL error


Reproducible: Always


User Profile Reset: No


OpenGL enabled: Yes

Additional Info:
Allow more Recordsets to be opened.  A counter that is Integer for RecordsetMax is too low, perhaps changing to Long.  I do not know if the Integer limit is carried into other tables or classes to make this practical.

The other option is to reuse OpenRecordset counters or at least provide a way to view the RecordsetMax variable as a property call for error trapping.  As it currently is the database cannot handle more complex uses without coding to its deficiencies.
Comment 1 tsultana 2018-11-11 03:22:30 UTC
Created attachment 146533 [details]
OpenRecordset error after opening and closing 32767 Recordset objects
Comment 2 tsultana 2018-11-11 03:25:06 UTC
The example code would have to be run 32768 times for the error to appear.  The x variable is not the cause, just the OpenRecordset action.
Comment 3 tsultana 2018-11-11 04:13:08 UTC
I mentioned using CurrentDb().CloseAllRecordsets() to reset the RecordsetMax = 0 but that does not work. Since I close each Recordset the Collection is empty so the routine exits without resetting the RecordsetMax variable. Below is the code for the CurrentDb().CloseAllRecordsets() subroutine which exits at the If IsNull() line.

Public Sub CloseAllRecordsets()
' Clean all recordsets for housekeeping

Dim sRecordsets() As String, i As Integer, oRecordset As Object
On Local Error Goto Exit_Sub

If IsNull(RecordsetsColl) Then Exit Sub
If RecordsetsColl.Count < 1 Then Exit Sub
For i = 1 To RecordsetsColl.Count
Set oRecordset = RecordsetsColl.Item(i)
oRecordset.mClose(False) ' Do not remove entry in collection
Next i
Set RecordsetsColl = New Collection
RecordsetMax = 0

Exit_Sub:
Exit Sub
End Sub ' CloseAllRecordsets V0.9.5

Changing the first two If statements to reset the RecordsetMax = 0 would fix the issue but may have other side effects that I am not aware of.
If IsNull(RecordsetsColl) Then
RecordsetMax = 0
Exit Sub
End If
If RecordsetsColl.Count < 1 Then
RecordsetMax = 0
Exit Sub
End If
Comment 4 Julien Nabet 2018-11-11 16:08:57 UTC
Jean-Pierre: thought you might be interested in this one.
Comment 5 tsultana 2018-11-11 17:11:27 UTC
I have worked around the bug in CloseAllRecordsets() by using the following code.

Dim recBug1 as Object
Dim recBug2 as Object

recBug1 = CurrentDb().OpenRecordset("table") ' needlessly open a recordset
recBug2 = CurrentDb().OpenRecordset("table") ' do it again
CurrentDb().CloseAllRecordsets()

This makes sure two recordsets are open in the collection to force the code to skip the If IsNull() and If .Count < 1 lines which then closes the two open recordsets and resets the RecordsetMax variable to 0.
Comment 6 Julien Nabet 2018-11-11 19:56:14 UTC
Created attachment 146541 [details]
Recordset.xba test

Thought this patch may help
diff --git a/wizards/source/access2base/Recordset.xba b/wizards/source/access2base/Recordset.xba
index cc46790532d9..f7193b22c65c 100644
--- a/wizards/source/access2base/Recordset.xba
+++ b/wizards/source/access2base/Recordset.xba
@@ -400,7 +400,10 @@ Dim i As Integer
        _Fields = Array()
        Set RowSet = Nothing
        If IsMissing(pbRemove) Then pbRemove = True
-       If pbRemove Then _ParentDatabase.RecordsetsColl.Remove(_Name)
+       If pbRemove Then
+               With _ParentDatabase
+                       .RecordsetMax = .RecordsetMax - 1
+                       .RecordsetsColl.Remove(_Name)

tony: you can give a try by retrieving the file and putting it in your profile (search in https://wiki.documentfoundation.org/UserProfile#Windows).
Comment 7 tsultana 2018-11-11 23:11:26 UTC
I installed the the Recordset to the /home/user/.config/libreoffice/4/user folder and restarted LO Base.

The following code was tested before the patch to confirm the same overflow error.  Running the same function after the patch crashes immediately after k = 32767 with the same error.

Sub test(v As Double)
	Dim rec As Object, k As Long 
	
	Do
		k = k + 1
		If k Mod 10000 = 0  or k Mod 32767 = 0 Then
			If msgbox("k = " & k, 1) = 2 Then
				Exit Do  
			End If 
		End If 
		rec = CurrentDb().OpenRecordset("configurations")
		rec.mClose
	Loop 
	CurrentDb().CloseAllRecordsets()
End Sub

Stepping into CloseAllRecordsets() in Access2Base.Database the RecordsetsColl.Count equals 0 and exits in the second If/Then below without clearing the RecordsetMax variable which overflowed.

Public Sub CloseAllRecordsets()
'	Clean all recordsets for housekeeping

Dim sRecordsets() As String, i As Integer, oRecordset As Object
	On Local Error Goto Exit_Sub

	If IsNull(RecordsetsColl) Then Exit Sub
	If RecordsetsColl.Count < 1 Then Exit Sub
Comment 8 tsultana 2018-11-11 23:14:44 UTC
THE VB IDE shows Access2Base.Database and Access2Base.Recordset have OpenRecordset() commands.  Is the patch calling the new one in Recordset or the old one in Database?
Comment 9 Jean-Pierre Ledure 2018-11-12 16:58:48 UTC
Hi,

I reproduced the issue with a variant of the script in Comment 7:

Sub TestBug121342()
	Dim db As Object, rec As Object, k As Long 
	Set db = Application.CurrentDb()
	db.CloseAllRecordsets()
	k = 0
	Do
		k = k + 1
		If k Mod 100 = 0 Then DoCmd.SysCmd(acSysCmdSetStatus, "k = " & k)
		Set rec = db.OpenRecordset("FACTURE")
		rec.mClose
	Loop 
	db.CloseAllRecordsets()
End Sub

Indeed the overflow is caused by the RecordsetMax variable which is an Integer and is,as a consequence, limited to 32767. Additionally the variable is not decremented when a recordset is closed, as should => This is a bug.

Replacing in the Database module of the Access2Base library the definition of RecordsetMax by replacing Integer by Long
- is a workaround for the current issue,
- has NO ADDITIONAL SIDE EFFECTS.

I suggest you to bypass the issue in your own code by adding something like
		If k Mod 100 = 0 Then db.RecordsetMax = 1
in the above loop. (1 and not 0 to force CloseAllRecordsets to do something even if the number of open recordsets in the same run is a multiple of 100 ...).

I will prepare a patch on master to make RecordsetMax a Long type and to include somewhere a decrement of its value.

JPL
Comment 10 Jean-Pierre Ledure 2018-11-13 07:31:22 UTC
THIS REPLACES COMMENT 9

I reproduced the issue with a variant of the script in Comment 7:

Sub TestBug121342()
	Dim db As Object, rec As Object, k As Long 
	Set db = Application.CurrentDb()
	db.CloseAllRecordsets()
	k = 0
	Do
		k = k + 1
		If k Mod 100 = 0 Then DoCmd.SysCmd(acSysCmdSetStatus, "k = " & k)
		Set rec = db.OpenRecordset("FACTURE")
		rec.mClose
	Loop 
	db.CloseAllRecordsets()
End Sub

Indeed the overflow is caused by the RecordsetMax variable which is an Integer and is,as a consequence, limited to 32767. That variable serves only to define a unique name for each entry in the Collection of recordsets. Each OpenRecordset() creates a new entry in the Collection with that number as name, each Recordset.mClose() deletes the entry without changing RecordsetMax.

Redefining in the Database module of the Access2Base library RecordsetMax from Integer to Long
- is a workaround for the current issue,
- has NO ADDITIONAL SIDE EFFECTS
- will give a maximum number of entries ~10M (name in Collection is formatted on 7 digits).

I suggest you to bypass the issue by patching your installation of LO with above  very limited change.

I will prepare a patch on master to make RecordsetMax a Long type.

JPL
Comment 11 Commit Notification 2018-11-13 08:13:28 UTC
Jean-Pierre Ledure committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/6f0dc4334a2772e14d8a1c771e9f31108212c506%5E%21

Access2Base - tdf#121342 Increase number of recordsets

It will be available in 6.2.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 12 Julien Nabet 2018-11-13 08:16:24 UTC
Jean-Pierre: following this patch, do you intend to submit a patch to decrement RecordMax when calling mbClose?
Comment 13 Jean-Pierre Ledure 2018-11-13 09:43:57 UTC
(In reply to Julien Nabet from comment #12)
> Jean-Pierre: following this patch, do you intend to submit a patch to
> decrement RecordMax when calling mbClose?

No, RecordsetMax should never be decremented. Its role is only to set a unique name in a Collection, it is not a counter of open Recordsets.

It is set to 0 when database is opened and incremented at each OpenRecordset(). The limit is now ~10M i.o. ~32K. The macro should run during weeks to reach this limit.
Even then, it is sufficient to close and reopen the database to start again from zero.

JPL
Comment 14 Julien Nabet 2018-11-13 10:05:04 UTC
(In reply to Jean-Pierre Ledure from comment #13)
> ...
> No, RecordsetMax should never be decremented. Its role is only to set a
> unique name in a Collection, it is not a counter of open Recordsets.
> ...
Thank you for the explanation