213512 - Sweep from nsIRDFPurgeableDataSource does not purge all the content!

Status: RESOLVED FIXED

(Core Graveyard :: RDF, defect)

Description

[Manel Mateos]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.2.1) Gecko/20021130
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.2.1) Gecko/20021130

if you Sweep a inMemoryDS with the following content, the main container is not
purged!

<?xml version="1.0"?>
<RDF:RDF xmlns:NS4="http://www.foo.com/rdfs/mb/mb.rdfs#"
         xmlns:NS3="http://www.foo.com/rdfs/be2c4d/be2c4d.rdfs#"
         xmlns:NS2="http://www.foo.com/rdfs/au/c4d.rdfs#"
         xmlns:NS1="http://www.foo.com/rdfs/fei/fei.rdfs#"
         xmlns:NC="http://home.netscape.com/NC-rdf#"
         xmlns:RDF="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_11"
                   NS4:WNOMBRE="Cia Reaseguro 2"
                   NS4:WCODIGO="63"
                   NS4:IDENTIDAD="www.foo.com-3F89808080808080808080808080-8480" />
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_3"
                   NS4:IDENTIDAD="www.foo.com-7B83808080808080808080808080-8380"
                   NS4:WCODIGO="05"
                   NS4:WNOMBRE="Compania Seguros Generales" />
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_9"
                   NS4:WCODIGO="55"
                   NS4:WNOMBRE="compaqma"
                   NS4:IDENTIDAD="www.foo.com-7A80808080808080808080808080-8780" />
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_13"
                   NS4:WCODIGO="69"
                   NS4:WNOMBRE="Cia 69"
                   NS4:IDENTIDAD="www.foo.com-5989808080808080808080808080-8080" />
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_5"
                   NS4:WCODIGO="10"
                   NS4:IDENTIDAD="www.foo.com-D28A808080808080808080808080-8280"
                   NS4:WNOMBRE="PRUEBAS Compania" />
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_12"
                   NS4:WNOMBRE="Cia Reaseguro 3"
                   NS4:WCODIGO="66"
                   NS4:IDENTIDAD="www.foo.com-3F89808080808080808080808080-8680" />
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_4"
                   NS4:WCODIGO="07"
                   NS4:WNOMBRE="TU TIA"
                   NS4:IDENTIDAD="www.foo.com-8F80808080808080808080808080-8C80" />
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_16"
                   NS4:emptyRecord="true" />
  <RDF:Seq about="http://www.foo.com/rdfs/mb/mb.rdfs#Query_pr_cargaCompanias">
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_0"/>
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_1"/>
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_2"/>
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_3"/>
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_4"/>
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_5"/>
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_6"/>
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_7"/>
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_8"/>
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_9"/>
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_10"/>
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_11"/>
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_12"/>
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_13"/>
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_14"/>
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_15"/>
    <RDF:li resource="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_16"/>
  </RDF:Seq>
  <NS4:Query about="http://www.foo.com/rdfs/mb/mb.rdfs#Query_pr_cargaCompanias"
                   NS4:id="pr_cargaCompanias"
                   NS4:viewID="pr_cargaCompanias" />
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_2"
                   NS4:IDENTIDAD="www.foo.com-6989808080808080808080808080-8080"
                   NS4:WCODIGO="03"
                   NS4:WNOMBRE="Cia pruebas" />
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_1"
                   NS4:WCODIGO="01"
                   NS4:WNOMBRE="Compania Seguros Generales"
                   NS4:IDENTIDAD="www.foo.com-AE86808080808080808080808080-8880"
                   NS2:selected="true" />
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_15"
                   NS4:WNOMBRE="Cia Reaseguro 4"
                   NS4:IDENTIDAD="www.foo.com-3F89808080808080808080808080-8880"
                   NS4:WCODIGO="97" />
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_0"
                   NS4:WCODIGO="00"
                   NS4:IDENTIDAD="www.foo.com-7186808080808080808080808080-EF80"
                   NS4:WNOMBRE="Compania" />
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_14"
                   NS4:WCODIGO="77"
                   NS4:WNOMBRE="Ocaso"
                   NS4:IDENTIDAD="www.foo.com-CC83808080808080808080808080-9280" />
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_10"
                   NS4:WNOMBRE="Cia Reaseguro 1"
                   NS4:IDENTIDAD="www.foo.com-3F89808080808080808080808080-8280"
                   NS4:WCODIGO="61" />
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_6"
                   NS4:IDENTIDAD="www.foo.com-3F89808080808080808080808080-8080"
                   NS4:WNOMBRE="Cedente de Reaseguro"
                   NS4:WCODIGO="36" />
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_7"
                   NS4:IDENTIDAD="www.foo.com-968B808080808080808080808080-8480"
                   NS4:WCODIGO="39"
                   NS4:WNOMBRE="CmpRehabila" />
  <RDF:Description about="http://www.foo.com/rdfs/mb/mb.rdfs#pr_cargaCompanias_8"
                   NS4:IDENTIDAD="www.foo.com-5489808080808080808080808080-8C80"
                   NS4:WCODIGO="40"
                   NS4:WNOMBRE="Compania Seguros-40" />
</RDF:RDF>


Reproducible: Always

Steps to Reproduce:

Actual Results:  
Memory increases and is not released!

Comment 1

[Axel Hecht]

confirmed. it seems like Sweep() only clears the assertions, but not the 
container data. It seems to me like this could leave wrong data after a 
nsRDFXMLDataSource.Refresh(), too. Which is the purpose of nsIPurgeableDataSource.

Comment 2

[Chase Tingley]

This WFM, but I think that's because I don't understand what the reporter is saying.

After chatting with Axel about what this in IRC, I tried the following
steps.

1.  Copy the above RDF into a file (sweep.rdf).
2.  Load the file in xpcshell
    var ds = rdfs.GetDataSourceBlocking("file:///file/tingley/sweep.rdf")

3.  Dump out the triples in the datasource.  (There are many)
4.  Open sweep.rdf, remove everything except for the root RDF:RDF node.
5.  Refresh the datasource
    var rds = ds.QueryInterface(Components.interfaces.nsIRDFRemoteDataSource)
    rds.Refresh(true)

6.  Dump out the triples in rds.  There are none.

This is the correct behavior.

As I told Axel, there are a whole mess of problems with the Purgeable
interface, but I don't understand this bug.

Comment 3

[Manel Mateos]

hi Chase! You should follow the following steps:

1.  Copy the RDF into a file (sweep.rdf).
2.  Load the file in xpcshell
    var ds = rdfs.GetDataSourceBlocking("file:///file/tingley/sweep.rdf")

3.  Sweep the datasource
    var rds = ds.QueryInterface(Components.interfaces.nsIRDFPurgeableDataSource)
    rds.Sweep()

4. All the statements have been removed but the container remains in the 
datasource.

Manel.

Comment 4

[Axel Hecht]

Attachment: testcase to be used in xpcshell, selfconsistent

Here is a testcase in a single file.
Startup xpcshell, load("testSweep.js") and find out what is still left in the
in-memory-ds. You may adjust serialize to work on your reload example.

Comment 5

[Chase Tingley]

Manel: 
> var rds = ds.QueryInterface(Components.interfaces.nsIRDFPurgeableDataSource)
This QI fails -- the RDFXMLDataSourceImpl does not implement
nsIRDFPurgeableDataSource.  (I think that it probably should, however.)

However, using Axel's example I finally see the problem.  And I think I know
what's wrong, too.  Patch incoming tonight -- taking this.

Thanks for your patience, Axel :)

Comment 6

[Chase Tingley]

I've got a patch that fixes the problem, but I want to sit on it for a day or
two to ensure the correctness and possibly clean up the code.

The source of this bug is rjc's EnsureFastContainment stuff from way back, which
is why the bug is sort of rare -- it only kicks in on a container with 7 or more
entries.  

http://lxr.mozilla.org/seamonkey/source/rdf/base/src/nsInMemoryDataSource.cpp#2076

Comment 7

[Chase Tingley]

Attachment: patch

This recursively invokes hash enumeration on the second-level property hashes,
should they exist.  (Note: in the current implementation, they will -only-
exist when the source is a container element that contains 7 or more items.)

When a second-level hash has been completely swept out, it is cleaned up
(rather than leaving it hanging empty off of mForwardArcs).  Then I went back
and added the same cleanup logic to LockedUnassert, to handle the case where
the hash empties out through a series of Unassert calls.

This code is somewhat more gruesome than I remembered.

Comment 8

[Robert John Churchill]

Comment on attachment 128607 [details] [diff] [review]
patch

r=[e-mail]

Comment 9

[Chase Tingley]

Comment on attachment 128607 [details] [diff] [review]
patch

Proactive denittifying: there are a couple |if (foo != nsnull)| tests that will
be changed to |if (!foo)|.

Comment 10

[Brendan Eich [:brendan]]

Comment on attachment 128607 [details] [diff] [review]
patch

Looks good to me.

/be

Comment 11

[Chase Tingley]

Woohoo!  Open trunk!  
Fix checked in.