bug#43146: 27.1; D-Bus property handling incomplete

[Hugh Daschbach]

When the same D-Bus property is registered on an interface with two
separate paths, only the last property is preserved. Methods and
signals registered on the same interface on different paths are
preserved.

The attached script demonstrates the property issue. It can be run as:

emacs -Q --load dbus-properties-test.el --eval "(main)"

The script registers a property on "/node0" then runs an asynchronous
dbus-send command to verify that the property is readable.

It then registers the same property on "/node1".

Subsequently, it attempts to read the property from both "/node0" and
"/node1". The property is successfully returned from the "/node1"
property get request. Nothing (boolean false) in returned from the
property get for "/node0".

Finally, it opens two windows, one displaying buffer "*node0*"
containing the command for the two get requests for "/node0". The first
is successful, the second is not.

The other window displays buffer "*node1* which shows the successful get
property on "/node1".

In short, the second registered property survives, the first does not.

[Michael Albinus]

Hugh Daschbach <hu...@ccss.com> writes:

Hi Hugh,

> When the same D-Bus property is registered on an interface with two
> separate paths, only the last property is preserved. Methods and
> signals registered on the same interface on different paths are
> preserved.

I could reproduce the bug locally. The following patch solves it for me,
could you pls check?

Best regards, Michael.

[Hugh Daschbach]

Michael Albinus writes:

> Hugh Daschbach <hu...@ccss.com> writes:
>
>> When the same D-Bus property is registered on an interface with
>> two
>> separate paths, only the last property is preserved. Methods
>> and
>> signals registered on the same interface on different paths are
>> preserved.
>
> I could reproduce the bug locally. The following patch solves it
> for me,
> could you pls check?

Michael, thank you for such a quick fix. I've tested this and it
does
indeed fix my issue.

Looking at how you fix it, I wonder if the change to
dbus-property-handler
should also apply to the "Set" and "GetAll" cond clauses.

Many thanks.
Hugh

[Hugh Daschbach]

Hugh Daschbach writes:

> Michael Albinus writes:
>
>> I could reproduce the bug locally. The following patch solves
>> it
>> for me,
>> could you pls check?
>
> Michael, thank you for such a quick fix. I've tested this and
> it
> does
> indeed fix my issue.
>
> Looking at how you fix it, I wonder if the change to
> dbus-property-handler
> should also apply to the "Set" and "GetAll" cond clauses.
>
> Many thanks.
> Hugh

I've tweaked the test script to examine the "Set" and "GetAll"
methods. The updated version is attached below.

Here's the sequence of operations:

/node0 /node1
------ ------
register prop, value "-node0-"
get prop, value is "-node0-"
register prop, value "-node1-"
get prop, value is "-node0-"
get prop, value is "-node1-"
set prop, value "-replaced-"
get prop, no value returned
get prop, value is "-replaced-"
getall, empty array returned

So I think the "Set" performed on /node1 corrupted the value of
the property no /node0. The last two operations on /node0 suggest
the value of /node0's property is no longer available.

Thanks again,
Hugh

[Michael Albinus]

Hugh Daschbach <hu...@ccss.com> writes:

Hi Hugh,

> Michael, thank you for such a quick fix. I've tested this and it
> does
> indeed fix my issue.
>
> Looking at how you fix it, I wonder if the change to
> dbus-property-handler
> should also apply to the "Set" and "GetAll" cond clauses.

I had exactly the same idea when I was in bed. Will do now.

> Many thanks.
> Hugh

Best regards, Michael.

[Michael Albinus]

Hugh Daschbach <hu...@ccss.com> writes:

Hi Hugh,

> I've tweaked the test script to examine the "Set" and "GetAll"
> methods. The updated version is attached below.

Thanks for the test file.

I've appended another patch, could you pls apply this instead of the
first one? Locally, I've tested your script successfully.

> Thanks again,
> Hugh

Best regards, Michael.

[Hugh Daschbach]

Michael Albinus writes:

> Hugh Daschbach <hu...@ccss.com> writes:
>
> Hi Hugh,
>
>> I've tweaked the test script to examine the "Set" and "GetAll"
>> methods. The updated version is attached below.
>
> Thanks for the test file.
>
> I've appended another patch, could you pls apply this instead of
> the
> first one? Locally, I've tested your script successfully.

That works! Thanks.

I've tested this with code that originally surfaced this issue and
no longer see a problem.

Please close the ticket at your convenience.

Thanks again.
Hugh

[Michael Albinus]

Version: 27.2

Hugh Daschbach <hu...@ccss.com> writes:

Hi Hugh,

>> I've appended another patch, could you pls apply this instead of the
>> first one? Locally, I've tested your script successfully.
>
> That works! Thanks.
>
> I've tested this with code that originally surfaced this issue and no
> longer see a problem.

Thanks for the feedback. I've pushed the patch to the emacvs-27 branch,
will appear with upcoming Emacs 27.2.

> Please close the ticket at your convenience.

Done. FYI, I use the opportunity to work on further dbus.el
enhancements, being on my todo for a long time. This includes the :write
access type of properties, and raising proper D-Bus error messages. Will
appear with Emacs 28.

> Thanks again.
> Hugh

Best regards, Michael.