RE: change of socketport constructor

[klaus triendl]

> Behalf Of Dimitar Dimitrov
> Sent: Tuesday, April 15, 2003 3:50 AM
> checked again in that "goto" loop. I don't know... may be a bool
> result will
> be good enough. expired() returns true and that's it, it really
> expired.. end

ok


> BTW, isn't it better if expired() is moved to TimePort interface.

to be discussed



> Some other remarks. They concern more Win32 implementation with
> which I played
> some time ago, so I may have forgotten something.
>
> For example something that becomes obvious very fast is the difference in
> semantic of the SocketService::Sync class and the pipe used in non-Win32
> implementation. The thing is that SocketService::Sync object has
> a place to
> hold only one byte. What happens at the end of ~SocketService().
> If there is
> no thread anymore (after update(0) and terminate() the thread
> should be gone)
> to read the flag from the Sync instance and to release the
> semaphore then the
> attempt to delete the next port object
>
> while(first)
>       delete first;
>
> will block because destructor of the port tries to call update()
> and that one
> blocks on the semaphore. Not to mention that the Sync object is already
> gone!!! BTW, the non-Win32 implementation doesn't look more correct, but
> there at least I can write to the pipe and my thread will not
> block. It also
> will not hurt if the pipe descriptors are closed at the end.

this is right and another thing with selfdeleting socketports also in
expired().. several calls to SocketService::update block (at least in
win32)!
also setTimer() (e.g. used in the example in ResetReadTimeout()) does not
need to update the SocketService because the timer is typically set in one
of the event methods and SocketService checks the timer after the event is
processed. i suggest to take out the parts of the code of SocketPort which
control the SocketService (setTimer(), ~SocketPort() ..) and that expired()
returns a bool whether the SocketPort deleted itself (as i did already).
as the SocketService controls the SocketPort and the two classes don't act
asynchronously this shouldn't be a problem, i think.



so, after all, what i would do for the patch is:
change ``SocketPort::SocketPort()" not to accept a service anymore;
change ``void SocketPort::expired()" in ``bool SocketPort::expired()";
don't proceed with ``retry:" and don't put the port into the event array if
expired() returns true
SocketService::~SocketService sets SocketPort::service = NULL before
deleting SocketPort
change SocketService::detach() not to update the service (is detaching and
attaching to another service ever needed?)
SocketPort::setTimer()+incTimer() don't update the service anymore
what about setDetectXXX()? should they still update the service?


opinions?


klaus