forked frompostgres/postgres
- Notifications
You must be signed in to change notification settings - Fork6
Commitf97de05
committed
Fix sloppy handling of corner-case errors in fd.c.
Several places in fd.c had badly-thought-through handling of error returnsfrom lseek() and close(). The fact that those would seldom fail on validFDs is probably the reason we've not noticed this up to now; but if theydid fail, we'd get quite confused.LruDelete and LruInsert actually just Assert'd that lseek never fails,which is pretty awful on its face.In LruDelete, we indeed can't throw an error, because that's likely to getcalled during error abort and so throwing an error would probably just leadto an infinite loop. But by the same token, throwing an error from theclose() right after that was ill-advised, not to mention that it would'veleft the LRU state corrupted since we'd already unlinked the VFD from thelist. I also noticed that really, most of the time, we should know thecurrent seek position and it shouldn't be necessary to do an lseek here atall. As patched, if we don't have a seek position and an lseek attemptdoesn't give us one, we'll close the file but then subsequent re-openattempts will fail (except in the somewhat-unlikely case that aFileSeek(SEEK_SET) call comes between and allows us to re-establish a knowntarget seek position). This isn't great but it won't result in any statecorruption.Meanwhile, having an Assert instead of an honest test in LruInsert isreally dangerous: if that lseek failed, a subsequent read or write wouldread or write from the start of the file, not where the caller expected,leading to data corruption.In both LruDelete and FileClose, if close() fails, just LOG that and markthe VFD closed anyway. Possibly leaking an FD is preferable to gettinginto an infinite loop or corrupting the VFD list. Besides, as far as I cantell from the POSIX spec, it's unspecified whether or not the file has beenclosed, so treating it as still open could be the wrong thing anyhow.I also fixed a number of other places that were being sloppy aboutbehaving correctly when the seekPos is unknown.Also, I changed FileSeek to return -1 with EINVAL for the cases where itdetects a bad offset, rather than throwing a hard elog(ERROR). It seemedpretty inconsistent that some bad-offset cases would get a failure returnwhile others got elog(ERROR). It was missing an offset validity check forthe SEEK_CUR case on a closed file, too.Back-patch to all supported branches, since all this code is fundamentallyidentical in all of them.Discussion:https://postgr.es/m/2982.1487617365@sss.pgh.pa.us1 parent3082098 commitf97de05
1 file changed
+136
-59
lines changedLines changed: 136 additions & 59 deletions
Original file line number | Diff line number | Diff line change | |
---|---|---|---|
| |||
160 | 160 |
| |
161 | 161 |
| |
162 | 162 |
| |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
163 | 169 |
| |
| 170 | + | |
164 | 171 |
| |
165 | 172 |
| |
166 | 173 |
| |
| |||
174 | 181 |
| |
175 | 182 |
| |
176 | 183 |
| |
177 |
| - | |
| 184 | + | |
178 | 185 |
| |
179 | 186 |
| |
180 | 187 |
| |
| |||
967 | 974 |
| |
968 | 975 |
| |
969 | 976 |
| |
970 |
| - | |
971 |
| - | |
972 |
| - | |
973 |
| - | |
974 |
| - | |
975 |
| - | |
| 977 | + | |
| 978 | + | |
| 979 | + | |
| 980 | + | |
| 981 | + | |
| 982 | + | |
| 983 | + | |
| 984 | + | |
| 985 | + | |
| 986 | + | |
| 987 | + | |
| 988 | + | |
| 989 | + | |
| 990 | + | |
| 991 | + | |
976 | 992 |
| |
977 |
| - | |
| 993 | + | |
| 994 | + | |
| 995 | + | |
| 996 | + | |
978 | 997 |
| |
979 |
| - | |
980 |
| - | |
981 |
| - | |
| 998 | + | |
982 | 999 |
| |
| 1000 | + | |
| 1001 | + | |
| 1002 | + | |
| 1003 | + | |
983 | 1004 |
| |
984 | 1005 |
| |
985 | 1006 |
| |
| |||
1030 | 1051 |
| |
1031 | 1052 |
| |
1032 | 1053 |
| |
1033 |
| - | |
| 1054 | + | |
1034 | 1055 |
| |
1035 | 1056 |
| |
1036 | 1057 |
| |
1037 | 1058 |
| |
1038 |
| - | |
1039 | 1059 |
| |
1040 | 1060 |
| |
1041 | 1061 |
| |
1042 |
| - | |
| 1062 | + | |
| 1063 | + | |
| 1064 | + | |
| 1065 | + | |
| 1066 | + | |
| 1067 | + | |
| 1068 | + | |
1043 | 1069 |
| |
1044 | 1070 |
| |
1045 |
| - | |
1046 |
| - | |
1047 |
| - | |
1048 |
| - | |
| 1071 | + | |
| 1072 | + | |
| 1073 | + | |
| 1074 | + | |
| 1075 | + | |
| 1076 | + | |
| 1077 | + | |
| 1078 | + | |
| 1079 | + | |
| 1080 | + | |
| 1081 | + | |
| 1082 | + | |
| 1083 | + | |
| 1084 | + | |
| 1085 | + | |
| 1086 | + | |
1049 | 1087 |
| |
1050 | 1088 |
| |
1051 | 1089 |
| |
| |||
1428 | 1466 |
| |
1429 | 1467 |
| |
1430 | 1468 |
| |
1431 |
| - | |
1432 |
| - | |
1433 |
| - | |
1434 | 1469 |
| |
1435 | 1470 |
| |
1436 |
| - | |
| 1471 | + | |
1437 | 1472 |
| |
1438 | 1473 |
| |
1439 | 1474 |
| |
| 1475 | + | |
| 1476 | + | |
| 1477 | + | |
1440 | 1478 |
| |
1441 | 1479 |
| |
1442 | 1480 |
| |
| |||
1566 | 1604 |
| |
1567 | 1605 |
| |
1568 | 1606 |
| |
| 1607 | + | |
1569 | 1608 |
| |
1570 | 1609 |
| |
1571 | 1610 |
| |
| |||
1578 | 1617 |
| |
1579 | 1618 |
| |
1580 | 1619 |
| |
| 1620 | + | |
| 1621 | + | |
1581 | 1622 |
| |
1582 |
| - | |
| 1623 | + | |
1583 | 1624 |
| |
1584 | 1625 |
| |
1585 |
| - | |
| 1626 | + | |
| 1627 | + | |
| 1628 | + | |
| 1629 | + | |
| 1630 | + | |
1586 | 1631 |
| |
1587 | 1632 |
| |
1588 | 1633 |
| |
| |||
1611 | 1656 |
| |
1612 | 1657 |
| |
1613 | 1658 |
| |
1614 |
| - | |
| 1659 | + | |
1615 | 1660 |
| |
1616 | 1661 |
| |
1617 | 1662 |
| |
| |||
1621 | 1666 |
| |
1622 | 1667 |
| |
1623 | 1668 |
| |
| 1669 | + | |
1624 | 1670 |
| |
1625 | 1671 |
| |
1626 | 1672 |
| |
| |||
1633 | 1679 |
| |
1634 | 1680 |
| |
1635 | 1681 |
| |
| 1682 | + | |
| 1683 | + | |
1636 | 1684 |
| |
1637 | 1685 |
| |
1638 | 1686 |
| |
| |||
1641 | 1689 |
| |
1642 | 1690 |
| |
1643 | 1691 |
| |
1644 |
| - | |
| 1692 | + | |
1645 | 1693 |
| |
1646 |
| - | |
| 1694 | + | |
1647 | 1695 |
| |
1648 |
| - | |
| 1696 | + | |
| 1697 | + | |
| 1698 | + | |
| 1699 | + | |
| 1700 | + | |
| 1701 | + | |
| 1702 | + | |
| 1703 | + | |
| 1704 | + | |
| 1705 | + | |
| 1706 | + | |
| 1707 | + | |
| 1708 | + | |
| 1709 | + | |
1649 | 1710 |
| |
1650 | 1711 |
| |
1651 | 1712 |
| |
1652 |
| - | |
| 1713 | + | |
1653 | 1714 |
| |
1654 | 1715 |
| |
1655 | 1716 |
| |
| |||
1660 | 1721 |
| |
1661 | 1722 |
| |
1662 | 1723 |
| |
1663 |
| - | |
| 1724 | + | |
1664 | 1725 |
| |
1665 | 1726 |
| |
1666 | 1727 |
| |
1667 | 1728 |
| |
1668 | 1729 |
| |
1669 | 1730 |
| |
1670 | 1731 |
| |
1671 |
| - | |
| 1732 | + | |
| 1733 | + | |
| 1734 | + | |
1672 | 1735 |
| |
1673 |
| - | |
1674 |
| - | |
| 1736 | + | |
| 1737 | + | |
| 1738 | + | |
| 1739 | + | |
| 1740 | + | |
| 1741 | + | |
| 1742 | + | |
| 1743 | + | |
1675 | 1744 |
| |
1676 |
| - | |
| 1745 | + | |
1677 | 1746 |
| |
1678 |
| - | |
| 1747 | + | |
1679 | 1748 |
| |
1680 |
| - | |
1681 |
| - | |
| 1749 | + | |
| 1750 | + | |
1682 | 1751 |
| |
1683 | 1752 |
| |
1684 | 1753 |
| |
| |||
1706 | 1775 |
| |
1707 | 1776 |
| |
1708 | 1777 |
| |
1709 |
| - | |
| 1778 | + | |
1710 | 1779 |
| |
1711 | 1780 |
| |
1712 | 1781 |
| |
| |||
1732 | 1801 |
| |
1733 | 1802 |
| |
1734 | 1803 |
| |
1735 |
| - | |
| 1804 | + | |
1736 | 1805 |
| |
1737 | 1806 |
| |
1738 | 1807 |
| |
| |||
1741 | 1810 |
| |
1742 | 1811 |
| |
1743 | 1812 |
| |
| 1813 | + | |
| 1814 | + | |
1744 | 1815 |
| |
1745 | 1816 |
| |
1746 | 1817 |
| |
1747 | 1818 |
| |
1748 | 1819 |
| |
1749 | 1820 |
| |
1750 |
| - | |
1751 |
| - | |
1752 |
| - | |
| 1821 | + | |
| 1822 | + | |
| 1823 | + | |
| 1824 | + | |
| 1825 | + | |
1753 | 1826 |
| |
1754 | 1827 |
| |
1755 |
| - | |
| 1828 | + | |
| 1829 | + | |
| 1830 | + | |
| 1831 | + | |
| 1832 | + | |
| 1833 | + | |
| 1834 | + | |
1756 | 1835 |
| |
1757 | 1836 |
| |
1758 |
| - | |
1759 |
| - | |
1760 |
| - | |
1761 |
| - | |
1762 |
| - | |
| 1837 | + | |
| 1838 | + | |
| 1839 | + | |
1763 | 1840 |
| |
1764 | 1841 |
| |
1765 | 1842 |
| |
| |||
1772 | 1849 |
| |
1773 | 1850 |
| |
1774 | 1851 |
| |
1775 |
| - | |
1776 |
| - | |
1777 |
| - | |
1778 |
| - | |
1779 |
| - | |
| 1852 | + | |
| 1853 | + | |
| 1854 | + | |
| 1855 | + | |
| 1856 | + | |
| 1857 | + | |
1780 | 1858 |
| |
1781 | 1859 |
| |
1782 |
| - | |
1783 |
| - | |
1784 |
| - | |
| 1860 | + | |
| 1861 | + | |
1785 | 1862 |
| |
1786 | 1863 |
| |
1787 |
| - | |
1788 |
| - | |
| 1864 | + | |
1789 | 1865 |
| |
1790 | 1866 |
| |
1791 | 1867 |
| |
1792 | 1868 |
| |
1793 | 1869 |
| |
1794 | 1870 |
| |
1795 |
| - | |
| 1871 | + | |
| 1872 | + | |
1796 | 1873 |
| |
1797 | 1874 |
| |
1798 | 1875 |
| |
|
0 commit comments
Comments
(0)