[Bug]: Sales Line-Reserve and Purch. Line-Reserve have infinite loop block
Open
Nobody has claimed this yet.
Team: SCM
- Dominant language
- AL
- Stars
- 683
- Forks
- 459
- Avg merge
- 3d 26m
- Merged PRs (30d)
- 633
Description
Describe the issue
In codeunit 99000832 Sales Line-Reserve there is procedure TransferSaleLineToSalesLine with loop for ReservationStatus := ReservationStatus::Reservation to ReservationStatus::Prospect do and loop if OldReservationEntry.FindSet() then repeat
Similarly, in codeunit 99000834 Purch. Line-Reserve there is TransferPurchLineToPurchLine with the same loop structure.
in certain conditions (when we have specific Reservation Entry records for Sales Blanket Order or Sales Quote (or potentially for any Purchase document type), the repeat loop is cycling without possibility to exit.
Expected behavior
The code continues without hanging infinitely with Reservation Entry locking.
Steps to reproduce
We had real life example, but I used mockup code to get similar Reservation Entries (do not beat me if I misused quantities)
usage: call the codeunit from any place
codeunit 50100 "FFD Test"
{
trigger OnRun()
var
FirstRun: Boolean;
RS: Enum "Reservation Status";
begin
InsertRecords();
Commit();
InsertEntries(RS::Prospect, RS::Surplus, 0, 1, 0, 1);
Commit();
RunCheck(); //!!! infinite loop
//!!! same behavior
// InsertEntries(RS::Prospect, RS::Surplus, 0, 1, 0, 0);
// Commit();
// RunCheck();
//!!! same behavior
// InsertEntries(RS::Surplus, RS::Prospect, 1, 0, 1, 0);
// Commit();
// RunCheck();
//!!! same behavior
// InsertEntries(RS::Surplus, RS::Prospect, 1, 0, 0, 0);
// Commit();
// RunCheck();
end;
/// <summary>
/// Inserting mockup Blanket Sales Order header and line; Sales Order header and line with specific description for easier finding
/// </summary>
local procedure InsertRecords()
var
SH: Record "Sales Header";
SL: Record "Sales Line";
Item: Record Item;
Customer: Record Customer;
begin
SH.SetRange("Posting Description", '!!! BSO');
if SH.IsEmpty() then begin
Item.SetRange(Type, Item.Type::Inventory);
Item.FindFirst();
Customer.SetRange(Blocked, Customer.Blocked::" ");
Customer.FindFirst();
SH.Init();
SH."Document Type" := SH."Document Type"::"Blanket Order";
SH.Validate("Sell-to Customer No.", Customer."No.");
SH.Insert(true);
SH."Posting Description" := '!!! BSO';
SH.Modify(true);
SL.Init();
SL.Validate("Document Type", SH."Document Type");
SL.Validate("Document No.", SH."No.");
SL."Line No." := 10000;
SL.Insert(true);
SL.Validate(Type, SL.Type::Item);
SL.Validate("No.", Item."No.");
SL.Validate(Quantity, 1);
SL.Description := '!!! BSO';
SL.Modify(true);
end;
SH.SetRange("Posting Description", '!!! SO');
if SH.IsEmpty() then begin
Item.SetRange(Type, Item.Type::Inventory);
Item.FindFirst();
Customer.SetRange(Blocked, Customer.Blocked::" ");
Customer.FindFirst();
SH.Init();
SH."Document Type" := SH."Document Type"::Order;
SH.Validate("Sell-to Customer No.", Customer."No.");
SH.Insert(true);
SH."Posting Description" := '!!! SO';
SH.Modify(true);
SL.Init();
SL.Validate("Document Type", SH."Document Type");
SL.Validate("Document No.", SH."No.");
SL."Line No." := 10000;
SL.Insert(true);
SL.Validate(Type, SL.Type::Item);
SL.Validate("No.", Item."No.");
SL.Validate(Quantity, 1);
SL.Description := '!!! SO';
SL.Modify(true);
end;
end;
/// <summary>
/// Inserting Reservation Entries with different reservation status, quantity and quantity to handle for the line inserted in InsertRecords procedure. The combination of parameters will determine the behavior of the loop in RunLoop procedure.
/// </summary>
/// <param name="RS1">Reservation Status of entry 1</param>
/// <param name="RS2">Reservation Status of entry 2</param>
/// <param name="Q1">Quantity of entry 1</param>
/// <param name="Q2">Quantity of entry 2</param>
/// <param name="H1">Quantity to handle of entry 1</param>
/// <param name="H2">Quantity to handle of entry 2</param>
local procedure InsertEntries(RS1: Enum "Reservation Status";
RS2: Enum "Reservation Status";
Q1: Decimal;
Q2: Decimal;
H1: Decimal;
H2: Decimal)
var
ResEntry: Record "Reservation Entry";
SL: Record "Sales Line";
EntryNo: Integer;
begin
//clearing up possible existing entries from previous runs, identified by the specific description on the sales line
SL.Reset();
SL.SetRange(Description, '!!! SO');
SL.FindFirst();
ResEntry.Reset();
ResEntry.SetRange("Source Type", Database::"Sales Line");
ResEntry.SetRange("Source Subtype", SL."Document Type".AsInteger());
ResEntry.SetRange("Source ID", SL."Document No.");
ResEntry.SetRange("Source Ref. No.", SL."Line No.");
ResEntry.DeleteAll();
SL.Reset();
SL.SetRange(Description, '!!! BSO');
SL.FindFirst();
ResEntry.Reset();
ResEntry.SetRange("Source Type", Database::"Sales Line");
ResEntry.SetRange("Source Subtype", SL."Document Type".AsInteger());
ResEntry.SetRange("Source ID", SL."Document No.");
ResEntry.SetRange("Source Ref. No.", SL."Line No.");
ResEntry.DeleteAll();
ResEntry.Reset();
if ResEntry.FindLast() then;
EntryNo := ResEntry."Entry No.";
// Inserting two entries with different reservation status, quantity and quantity to handle for the same sales line
ResEntry.Init();
ResEntry."Entry No." := EntryNo + 1;
ResEntry."Source Type" := Database::"Sales Line";
ResEntry."Source Subtype" := SL."Document Type".AsInteger();
ResEntry."Source ID" := SL."Document No.";
ResEntry."Source Ref. No." := SL."Line No.";
ResEntry."Item No." := SL."No.";
ResEntry."Quantity (Base)" := Q1;
ResEntry.Quantity := ResEntry."Quantity (Base)";
ResEntry."Qty. to Handle (Base)" := H1;
ResEntry."Qty. to Invoice (Base)" := H1;
ResEntry."Reservation Status" := RS1;
ResEntry.Insert(false);
ResEntry.Init();
ResEntry."Entry No." := EntryNo + 2;
ResEntry."Source Type" := Database::"Sales Line";
ResEntry."Source Subtype" := SL."Document Type".AsInteger();
ResEntry."Source ID" := SL."Document No.";
ResEntry."Source Ref. No." := SL."Line No.";
ResEntry."Item No." := SL."No.";
ResEntry."Quantity (Base)" := Q2;
ResEntry.Quantity := ResEntry."Quantity (Base)";
ResEntry."Qty. to Handle (Base)" := H2;
ResEntry."Qty. to Invoice (Base)" := H2;
ResEntry."Reservation Status" := RS2;
ResEntry.Insert(false);
end;
/// <summary>
/// Running the check for created entries.
/// </summary>
local procedure RunCheck()
var
OldSalesLine: Record "Sales Line";
NewSalesLine: Record "Sales Line";
TransferQty: Decimal;
begin
OldSalesLine.SetRange(Description, '!!! BSO');
OldSalesLine.FindFirst();
NewSalesLine.SetRange(Description, '!!! SO');
NewSalesLine.FindFirst();
TransferQty := OldSalesLine.Quantity;
TransferSaleLineToSalesLine(OldSalesLine, NewSalesLine, TransferQty);
end;
/// <summary>
/// Almost copy-paste from "Sales Line-Reserve".TransferSaleLineToSalesLine, with skipping checks and logic not relevant for the test, to be able to reproduce the issue in a loop.
/// </summary>
/// <param name="OldSalesLine">same parameters as in original function</param>
/// <param name="NewSalesLine">same parameters as in original function</param>
/// <param name="TransferQty">same parameters as in original function</param>
local procedure TransferSaleLineToSalesLine(var OldSalesLine: Record "Sales Line"; var NewSalesLine: Record "Sales Line"; TransferQty: Decimal)
var
OldReservationEntry: Record "Reservation Entry";
ReservationStatus: Enum "Reservation Status";
CreateReservEntry: Codeunit "Create Reserv. Entry";
SalesLineReserve: Codeunit "Sales Line-Reserve";
begin
// block from "Sales Line-Reserve".TransferSaleLineToSalesLine, skipping unnecessary checks
if not SalesLineReserve.FindReservEntry(OldSalesLine, OldReservationEntry) then
exit;
OldReservationEntry.Lock();
for ReservationStatus := ReservationStatus::Reservation to ReservationStatus::Prospect do begin
if TransferQty = 0 then
exit;
OldReservationEntry.SetRange("Reservation Status", ReservationStatus);
if OldReservationEntry.FindSet() then
repeat
OldReservationEntry.TestItemFields(OldSalesLine."No.", OldSalesLine."Variant Code", OldSalesLine."Location Code");
if (OldReservationEntry."Reservation Status" = OldReservationEntry."Reservation Status"::Prospect) and
(OldSalesLine."Document Type" in [OldSalesLine."Document Type"::Quote,
OldSalesLine."Document Type"::"Blanket Order"])
then
OldReservationEntry."Reservation Status" := OldReservationEntry."Reservation Status"::Surplus;
CreateReservEntry.TransferReservEntry(
Database::"Sales Line",
NewSalesLine."Document Type".AsInteger(), NewSalesLine."Document No.", '', 0,
NewSalesLine."Line No.", NewSalesLine."Qty. per Unit of Measure", OldReservationEntry, TransferQty);
until (OldReservationEntry.Next() = 0) or (TransferQty = 0);
end;
end;
}
Additional context
The fix is simple - introduce new variable PassReservationEntry: Record "Reservation Entry", assign PassReservationEntry := OldReservationEntry;, and use it for changing and passing to the CreateReservEntry.TransferReservEntry, instead of OldReservationEntry.
Current code of codeunit 99000832 "Sales Line-Reserve":
procedure TransferSaleLineToSalesLine(var OldSalesLine: Record "Sales Line"; var NewSalesLine: Record "Sales Line"; TransferQty: Decimal)
var
OldReservationEntry: Record "Reservation Entry";
ReservationStatus: Enum "Reservation Status";
IsHandled: Boolean;
begin
// Used for sales quote and blanket order when transferred to order
IsHandled := false;
OnBeforeTransferSaleLineToSalesLine(OldSalesLine, NewSalesLine, TransferQty, IsHandled);
if IsHandled then
exit;
if not FindReservEntry(OldSalesLine, OldReservationEntry) then
exit;
OldReservationEntry.Lock();
NewSalesLine.TestItemFields(OldSalesLine."No.", OldSalesLine."Variant Code", OldSalesLine."Location Code");
for ReservationStatus := ReservationStatus::Reservation to ReservationStatus::Prospect do begin
if TransferQty = 0 then
exit;
OldReservationEntry.SetRange("Reservation Status", ReservationStatus);
if OldReservationEntry.FindSet() then
repeat
OldReservationEntry.TestItemFields(OldSalesLine."No.", OldSalesLine."Variant Code", OldSalesLine."Location Code");
if (OldReservationEntry."Reservation Status" = OldReservationEntry."Reservation Status"::Prospect) and
(OldSalesLine."Document Type" in [OldSalesLine."Document Type"::Quote,
OldSalesLine."Document Type"::"Blanket Order"])
then
OldReservationEntry."Reservation Status" := OldReservationEntry."Reservation Status"::Surplus;
IsHandled := false;
OnTransferSaleLineToSalesLineOnBeforeCalcTransferQty(NewSalesLine, OldReservationEntry, IsHandled);
if not IsHandled then
TransferQty :=
CreateReservEntry.TransferReservEntry(
Database::"Sales Line",
NewSalesLine."Document Type".AsInteger(), NewSalesLine."Document No.", '', 0,
NewSalesLine."Line No.", NewSalesLine."Qty. per Unit of Measure", OldReservationEntry, TransferQty);
until (OldReservationEntry.Next() = 0) or (TransferQty = 0);
end;
end;
Proposed change
procedure TransferSaleLineToSalesLine(var OldSalesLine: Record "Sales Line"; var NewSalesLine: Record "Sales Line"; TransferQty: Decimal)
var
OldReservationEntry: Record "Reservation Entry";
PassReservationEntry: Record "Reservation Entry"; //new variable
ReservationStatus: Enum "Reservation Status";
IsHandled: Boolean;
begin
// Used for sales quote and blanket order when transferred to order
IsHandled := false;
OnBeforeTransferSaleLineToSalesLine(OldSalesLine, NewSalesLine, TransferQty, IsHandled);
if IsHandled then
exit;
if not FindReservEntry(OldSalesLine, OldReservationEntry) then
exit;
OldReservationEntry.Lock();
NewSalesLine.TestItemFields(OldSalesLine."No.", OldSalesLine."Variant Code", OldSalesLine."Location Code");
for ReservationStatus := ReservationStatus::Reservation to ReservationStatus::Prospect do begin
if TransferQty = 0 then
exit;
OldReservationEntry.SetRange("Reservation Status", ReservationStatus);
if OldReservationEntry.FindSet() then
repeat
PassReservationEntry := OldReservationEntry; //assignment
PassReservationEntry.TestItemFields(OldSalesLine."No.", OldSalesLine."Variant Code", OldSalesLine."Location Code"); //use in checking
if (PassReservationEntry."Reservation Status" = PassReservationEntry."Reservation Status"::Prospect) and //use in checking
(OldSalesLine."Document Type" in [OldSalesLine."Document Type"::Quote,
OldSalesLine."Document Type"::"Blanket Order"])
then
PassReservationEntry."Reservation Status" := PassReservationEntry."Reservation Status"::Surplus; //use in changing
IsHandled := false;
OnTransferSaleLineToSalesLineOnBeforeCalcTransferQty(NewSalesLine, PassReservationEntry, IsHandled); //use in publisher
if not IsHandled then
TransferQty :=
CreateReservEntry.TransferReservEntry(
Database::"Sales Line",
NewSalesLine."Document Type".AsInteger(), NewSalesLine."Document No.", '', 0,
NewSalesLine."Line No.", NewSalesLine."Qty. per Unit of Measure", PassReservationEntry, TransferQty); //use in passing
until (OldReservationEntry.Next() = 0) or (TransferQty = 0);
end;
end;
similar change in codeunit 99000834 "Purch. Line-Reserve"
I will provide a fix for a bug
- I will provide a fix for a bug
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with TransferSaleLineToSalesLine in codeunit 99000832 "Sales Line-Reserve" and compare the corresponding procedure in codeunit 99000834 "Purch. Line-Reserve". Run the provided codeunit 50100 reproduction with the listed Reservation Entry combinations. Done means the transfer completes without an infinite loop or hanging on Reservation Entry locks in both sales and purchase paths.
Written by the indexing model from the issue text.
Assessment
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100