Re: [certi-dev] RFC: avoid racy sleep to establish connection to rtia

[Eric Noulard]

2009/8/10 Mathias Fröhlich <address@hidden>:
>
> Hi all,

Hi Matthias,
Thank you very much for your documentted feedback & patch.

> I am Mathias Fröhlich, working in my spare time for FlightGear.
> My aim is to have some working code in FlightGear that makes use of certi as
> HLA/RTI component.
>
> I have done a patch to certi which avoids the racy sleep() when starting up
> rtia:

You are right the sleep was ugly.

> * On UNIX, the default way to set up this connection is changed to use the
> socketpair library call to establish the two way connection to rtia.
> * As a side effect of the above we do no longer need to care for cleaning up
> the named pipe file on UNIX which occasionally was not cleaned up in my test
> environment.

Nice and clear idea.

> * On win32 the connection is still done by a local tcp socket, but for this
> case the role of the server and client is reversed to avoid the race
> condition. The application is acting as the server and the rtia process
> connects to the listening server socket in the application.

This may be a security breach, because RTIA used to be a trusted process,
for which you can authorize server port listening. The situation is not that
good if we make the server reside in the application,
but may be we can live with it, this has to be discussed.

> * The TCP port number does no longer default to the process id which should
> eliminate some possible failures due to process id's that are equal to already
> occupied port numbers.

Ok, then I'll see how you did it in your patch.

> * Signals are unblocked  when a problem appears in the fork/exec path when
> starting up rtia.
> * Open files are closed in the rtia childs process past the fork.
>
> I have tested the attached patch on fedora linux 32 and 64 bits as well as on
> win32 and win64.
> I have done compile tests on all our unix machines I have access to at work.
> This includes at least AIX5, Solaris-8, an ancient IRIX variant and HP-
> UX-11.11.

That sort of thorough testing, thank you very much for that.

> Note that on some of the architectures certi does not compile as a whole, but
> the changed files do compile.

We may work together to make CERTI compile as whole on non-working machine
if you want and have time for that.

> The patch should apply on todays cvs.

Very good.

> Please include that changes with the next release of certi or tell me what I
> need to do to get that or something similar into certi.

I'll review your patch and then tell you more about it.
Note that the usual way to submit a patch is to create
a new entry in the patch tracker:
https://savannah.nongnu.org/patch/?group=certi

You should do that while properly logged-in with your Savannah account
this way:

 1 - The submitter (you) get automatic follow-up
 2 - We (CERTI dev & admin) can easily track pending patches

Announcing the provided patch on the ML is good too in order to make
the community aware of it. Admin and interested CERTI dev do already
get notification for each new bug/patches, but adding an explaining message
on the list pointing to the supplied patch tracker entry is good too.

May be you can already add your patch to the patch tracker?

-- 
Erk
Membre de l'April - « promouvoir et défendre le logiciel libre » -
http://www.april.org