Is psSocketPoll doing the right thing?
Hello.
While looking a patch, I found that pqSocketPoll passes through the
result from poll(2) to the caller and throws away revents. If I
understand it correctly, poll() *doesn't* return -1 nor errno by the
reason it has set POLLERR, POLLHUP, POLLNVAL, and POLLRDHUP for some
of the target sockets, and returns 0 unless poll() itself failed to
work.
It doesn't seem to be the intended behavior since the function sets
POLLERR to pollfd.events. (but the bit is ignored by poll(), though)
Is the above diagnosis correct?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On 2023/02/09 11:50, Kyotaro Horiguchi wrote:
Hello.
While looking a patch, I found that pqSocketPoll passes through the
result from poll(2) to the caller and throws away revents. If I
understand it correctly, poll() *doesn't* return -1 nor errno by the
reason it has set POLLERR, POLLHUP, POLLNVAL, and POLLRDHUP for some
of the target sockets, and returns 0 unless poll() itself failed to
work.
As far as I understand correctly, poll() returns >0 if "revents"
has either of those bits, not 0 nor -1.
You're thinking that pqSocketPoll() should check "revents" and
return -1 if either of those bits is set?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Subject: is p*s*Socket..
Oops...
At Thu, 9 Feb 2023 17:32:08 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
On 2023/02/09 11:50, Kyotaro Horiguchi wrote:
Hello.
While looking a patch, I found that pqSocketPoll passes through the
result from poll(2) to the caller and throws away revents. If I
understand it correctly, poll() *doesn't* return -1 nor errno by the
reason it has set POLLERR, POLLHUP, POLLNVAL, and POLLRDHUP for some
of the target sockets, and returns 0 unless poll() itself failed to
work.As far as I understand correctly, poll() returns >0 if "revents"
has either of those bits, not 0 nor -1.
Right. as my understanding.
If any of the sockets is in any of the states, pqSocketPoll returns a
positive, which makes pqSocketCheck return 1. Finally
pqRead/WriteReady return "ready" even though the connection socket is
in an error state. Actually that behavior doesn't harm since the
succeeding actual read/write will "properly" fail. However, once we
use this function to simply check the socket is sound without doing an
actual read/write, that behavior starts giving a harm by the false
answer.
You're thinking that pqSocketPoll() should check "revents" and
return -1 if either of those bits is set?
In short, yes.
pqSocketPoll() should somehow inform callers about that
state. Fortunately pqSocketPoll is a private function thus we can
refactor the function so that it can do that properly.
If no one object to that change, I'll work on that.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On 2023-02-10 10:42, Kyotaro Horiguchi wrote:
On 2023/02/09 11:50, Kyotaro Horiguchi wrote:
Hello.
While looking a patch, I found that pqSocketPoll passes through the
result from poll(2) to the caller and throws away revents. If I
understand it correctly, poll() *doesn't* return -1 nor errno by the
reason it has set POLLERR, POLLHUP, POLLNVAL, and POLLRDHUP for some
of the target sockets, and returns 0 unless poll() itself failed to
work.As far as I understand correctly, poll() returns >0 if "revents"
has either of those bits, not 0 nor -1.Right. as my understanding.
If any of the sockets is in any of the states, pqSocketPoll returns a
positive, which makes pqSocketCheck return 1. Finally
pqRead/WriteReady return "ready" even though the connection socket is
in an error state. Actually that behavior doesn't harm since the
succeeding actual read/write will "properly" fail. However, once we
use this function to simply check the socket is sound without doing an
actual read/write, that behavior starts giving a harm by the false
answer.
I agree with you. Current pqScoketCheck could return a false result
from a caller's point of view.
You're thinking that pqSocketPoll() should check "revents" and
return -1 if either of those bits is set?In short, yes.
pqSocketPoll() should somehow inform callers about that
state. Fortunately pqSocketPoll is a private function thus we can
refactor the function so that it can do that properly.
Does this mean that pqSocketPoll or pqSocketCheck somehow returns the
poll's result including error conditions (POLLERR, POLLHUP, POLLNVAL)
to callers? Then callers filter the result to make their final result.
regards,
--
Katsuragi Yuta
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Dear Horiguchi-san,
My comment may be no longer needed, but I can +1 to your opinion.
On 2023/02/09 11:50, Kyotaro Horiguchi wrote:
Hello.
While looking a patch, I found that pqSocketPoll passes through the
result from poll(2) to the caller and throws away revents. If I
understand it correctly, poll() *doesn't* return -1 nor errno by the
reason it has set POLLERR, POLLHUP, POLLNVAL, and POLLRDHUP forsome
of the target sockets, and returns 0 unless poll() itself failed to
work.As far as I understand correctly, poll() returns >0 if "revents"
has either of those bits, not 0 nor -1.Right. as my understanding.
If any of the sockets is in any of the states, pqSocketPoll returns a
positive, which makes pqSocketCheck return 1. Finally
pqRead/WriteReady return "ready" even though the connection socket is
in an error state. Actually that behavior doesn't harm since the
succeeding actual read/write will "properly" fail. However, once we
use this function to simply check the socket is sound without doing an
actual read/write, that behavior starts giving a harm by the false
answer.
I checked man page of poll(3), and it said that POLLERR, POLLHUP, POLLNVAL is only
valid in revents. Moreover, poll() has is clarified that it returns natural number
if revent is larger than zero. So revent should be checked even if the returned
value > 0.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED