MakeImmutable fails with error "Argument has inaccessible subobjects"

[mohamed-barakat]

MakeImmutable breaks the backwards compatibility of HPCGAP with GAP4:

gap> LoadPackage( "MatricesForHomalg" );
true
gap> ZZ := HomalgRingOfIntegers( );
Z
gap> m := HomalgInitialMatrix( 2, 2, ZZ );
<An initial 2 x 2 matrix over an internal ring>
gap> InfoOfObject( m );
rec( attributes := rec( NrColumns := 2, NrRows := 2 ), components := rec( ring := Z ),
  object := <An initial 2 x 2 matrix over an internal ring>, properties := rec( IsEmptyMatrix := false, IsInitialMatrix := true ) )
gap> MakeImmutable( m );
Error, MakeImmutable: Argument has inaccessible subobjects
not in any function at line 5 of stream

[fingolfin]

Actually, m in this situation is already immutable. And then MakeImmutable should be an idempotent. But it isn't, due to checks performed in the kernel. Specifically, PreMakeImmutableCheck checks whether we have exclusive write access to the object (which we don't, as it lives in the public region, which is fine, given that it is immutable).

I guess this means the kernel should first check whether the object already is immutable; if it is, do nothing and return.

[fingolfin]

One fix would be to modify CheckedMakeImmutable (which generates the error we are seeing here) to call IS_MUTABLE_OBJ, like it sibling MakeImmutable does. But I am not sure if this would have bad side effects... :-/

@rbehrends Any comments?

[rbehrends]

The problem is slightly more complicated. PreMakeImmutableCheck() already tests if the top-level object is immutable, and CheckedMakeImmutable() then just calls MakeImmutable() if the check passes.

What we're seeing seems to happen (I think) because m is an atomic component object and mutability here implies that RESET_FILTER_OBJ() needs to be called (same for atomic positional objects). We need to special-case these tnums both in the traversal logic and in the MakeImmutable() table (so that the filter update is actually thread-safe in case two threads try to do this simultaneously).

A related problem that remains is how to handle MakeImmutable() for T_DATOBJ; we've discussed this before: a possible solution would be to use MakeReadOnly() underneath, but that would require some additional hackage in the kernel.

[stevelinton]

Just run into this again in #3592 My test code makes a mutable attribute storing object and then makes it immutable with MakeImmutable. That gives this error, even when the object actually has no subobjects (except its type, which I assume is the offender here).